HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Thread-safety and delegates with generated numbers

Submitted by: @import:stackexchange-codereview··
0
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

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.

Context

StackExchange Code Review Q#4890, answer score: 3

Revisions (0)

No revisions yet.