Is the following C # code stream safe?

Question

I am trying to learn threads in C #. Today I am sowing the following code at http://www.albahari.com/threading/ :

class ThreadTest { bool done; static void Main() { ThreadTest tt = new ThreadTest(); // Create a common instance new Thread (tt.Go).Start(); tt.Go(); } // Note that Go is now an instance method void Go() { if (!done) { done = true; Console.WriteLine ("Done"); } } } 

In Java, if you do not define “done” as mutable, the code will not be safe. How does the C # memory model handle this?

Guys, Thanks everyone for the answers. Very grateful.

Answer 1

Well, there is a clear race condition that they could see done as false and execute the if body - this is true, regardless of the memory model. Running done volatile will not fix this, and it will not fix it in Java either.

But yes, it is possible that changes made in one thread may occur, but will not be visible until in another thread. It depends on the processor architecture, etc. As an example of what I mean, consider this program:

 using System; using System.Threading; class Test { private bool stop = false; static void Main() { new Test().Start(); } void Start() { new Thread(ThreadJob).Start(); Thread.Sleep(500); stop = true; } void ThreadJob() { int x = 0; while (!stop) { x++; } Console.WriteLine("Counted to {0}", x); } } 

While this ends on my current laptop, I used other machines where pretty much the same code would work forever — it would never “see” the change to stop in the second thread.

Basically, I try to avoid writing code without blocking, unless it uses the higher-level abstractions provided by people who really know their stuff - for example, Parallel Extensions in .NET 4.

There is a way to make this code open and correct using Interlocked . For instance:

 class ThreadTest { int done; static void Main() { ThreadTest tt = new ThreadTest(); // Create a common instance new Thread (tt.Go).Start(); tt.Go(); } // Note that Go is now an instance method void Go() { if (Interlocked.CompareExchange(ref done, 1, 0) == 0) { Console.WriteLine("Done"); } } } 

Here, the value change and its testing are performed as a whole: CompareExchange will set the value to only 1 if it is currently 0, and will return the old value. Thus, only one thread will ever see the return value of 0.

One more thing to keep in mind: your question is rather ambiguous, as you have not defined what you mean by “streaming security”. I guessed about your intentions, but you never made it clear. Read this Eric Lippert blog post - it's worth it.

Answer 2

No, it is not thread safe. Perhaps you have one thread checking the condition ( if(!done) ), another thread checking the same condition, and then the first thread executes the first line in the code block ( done = true ).

You can make it thread safe with locking:

 lock(this) { if(!done) { done = true; Console.WriteLine("Done"); } } 

Answer 3

Even in Java with volatile both threads can enter a block using WriteLine .

If you need a mutual exception, you need to use a real synchronization object, such as a lock.

Answer 4

onle way is thread safe when you use atomic comparison and set if tag

 if(atomicBool.compareAndSet(false,true)){ Console.WriteLine("Done"); } 

Answer 5

You should do something like this:

 class ThreadTest{ Object myLock = new Object(); ... void Go(){ lock(myLock){ if(!done) { done = true; Console.WriteLine("Done"); } } } 

The reason you want to use a shared object rather than "this" is because if your object (aka "this") changes at all, it is considered another object. Thus, your castle no longer works.

Another little thing you might think. This is “good practice,” so nothing serious.

 class ThreadTest{ Object myLock = new Object(); ... void Go(){ lock(myLock){ if(!done) { done = true; } } //This line of code does not belong inside the lock. Console.WriteLine("Done"); } 

Never use a code inside a lock that should not be inside a lock. This is due to the delay caused by this. If you have many threads, you can get more performance by removing all this unnecessary waiting.

Hope this helps :)