patterncsharpMinor
Thread-safety and delegates with generated numbers
Viewed 0 times
generatedwithnumbersthreaddelegatessafetyand
Problem
In the main class, loops generate numbers (0~100), and when its generated number is > 20, its value is passed to the thread where it simulates some work with this number. Meanwhile, while this number is being processed, other generated numbers > 20 must be skipped.
Is this code OK? I am not sure if I'm doing something bad or not. In fact it seems to be working fine, but I don't know if it can be written by that way or even "better.".
```
class Program
{
delegate void SetNumberDelegate(int number);
delegate bool IsBusyDelegate();
static void Main(string[] args)
{
Random rnd = new Random();
ProcessingClass processClass = new ProcessingClass();
SetNumberDelegate setNum = new SetNumberDelegate(processClass.setNumber);
IsBusyDelegate isBusy = new IsBusyDelegate(processClass.isBusy);
Thread processThread = new Thread(new ThreadStart(processClass.processNumbers));
processThread.Start();
int num;
int count = 0;
while (count++ 20)
{
if (!isBusy())
{
setNum(num);
}
else
{
Console.WriteLine("Thread BUSY! Skipping number:{0}", num);
}
}
Thread.Sleep(1);
}
processThread.Abort();
processThread.Join();
Console.ReadKey();
}
}
class ProcessingClass
{
private volatile bool busy;
private volatile int number;
public ProcessingClass()
{
busy = false;
number = -1;
}
public bool isBusy()
{
return busy;
}
public void setNumber(int num)
{
number = num;
}
public void processNumbers()
{
while (true)
{
if (number > -1 && !busy)
{
busy = true;
Console.WriteLine("Processing number:{0}", number);
// simulate some work with number
Is this code OK? I am not sure if I'm doing something bad or not. In fact it seems to be working fine, but I don't know if it can be written by that way or even "better.".
```
class Program
{
delegate void SetNumberDelegate(int number);
delegate bool IsBusyDelegate();
static void Main(string[] args)
{
Random rnd = new Random();
ProcessingClass processClass = new ProcessingClass();
SetNumberDelegate setNum = new SetNumberDelegate(processClass.setNumber);
IsBusyDelegate isBusy = new IsBusyDelegate(processClass.isBusy);
Thread processThread = new Thread(new ThreadStart(processClass.processNumbers));
processThread.Start();
int num;
int count = 0;
while (count++ 20)
{
if (!isBusy())
{
setNum(num);
}
else
{
Console.WriteLine("Thread BUSY! Skipping number:{0}", num);
}
}
Thread.Sleep(1);
}
processThread.Abort();
processThread.Join();
Console.ReadKey();
}
}
class ProcessingClass
{
private volatile bool busy;
private volatile int number;
public ProcessingClass()
{
busy = false;
number = -1;
}
public bool isBusy()
{
return busy;
}
public void setNumber(int num)
{
number = num;
}
public void processNumbers()
{
while (true)
{
if (number > -1 && !busy)
{
busy = true;
Console.WriteLine("Processing number:{0}", number);
// simulate some work with number
Solution
There is no synchronisation here and so all access to the thread's state is subject to data races. It is comprehensively not threadsafe.
From what I can tell all you need is to implement a producer/consumer pattern. Do this with a blocking queue. Please don't use busy loops. They burn CPU cycles and are horribly non-performant.
From what I can tell all you need is to implement a producer/consumer pattern. Do this with a blocking queue. Please don't use busy loops. They burn CPU cycles and are horribly non-performant.
Context
StackExchange Code Review Q#4890, answer score: 3
Revisions (0)
No revisions yet.