19

When I run the following code in debug mode, it'll successfully finish and exit. However, if I run the following code in release mode, it'll get stuck in an infinite loop and never finish.

static void Main(string[] args)
{
    bool stop = false;

    new Thread(() =>
    {
        Thread.Sleep(1000);
        stop = true;
        Console.WriteLine("Set \"stop\" to true.");

    }).Start();

    Console.WriteLine("Entering loop.");

    while (!stop)
    {
    }

    Console.WriteLine("Done.");
}

Which optimization is causing it to get stuck in an infinite loop?

Martin Brown
  • 24,692
  • 14
  • 77
  • 122
nivlam
  • 3,223
  • 4
  • 30
  • 39

4 Answers4

18

My guess would be processor caching of the stop variable on the main thread. In debug mode the memory model is stricter because the debugger needs to be able to provide a sensible view of the variable's state across all threads.

Try making a field and marking it as volatile:

volatile bool stop = false;

static void Main(string[] args)
{

    new Thread(() =>
    {
        Thread.Sleep(1000);
        stop = true;
        Console.WriteLine("Set \"stop\" to true.");

    }).Start();

    Console.WriteLine("Entering loop.");

    while (!stop)
    {
    }

    Console.WriteLine("Done.");
}
Chris Shain
  • 50,833
  • 6
  • 93
  • 125
  • Volatile is good for correctness, but to avoid optimization it look like making it field instead of local variable is enough. – Alexei Levenkov May 25 '12 at 04:50
  • @AlexeiLevenkov I could be mistaken but I believe that is an implementation detail of the CLR, and theoretically (though probably not practically) subject to change. – Chris Shain May 25 '12 at 04:57
  • I wonder why you are not synchronizing the access to the variable. I always do this, since I assume it is not an atomic operation to get/set a `bool` variable. Am I wrong on this? – Uwe Keim May 25 '12 at 05:06
  • 1
    I think it's going to be quite hard to figure out a sensible non-atomic way to set or get a boolean variable... Caching (either by the CPU or the compiler itself) is the cause for this. – zmbq May 25 '12 at 06:01
  • 2
    @UweKeim: get/set of a bool is definitely atomic. It's in the spec. http://stackoverflow.com/questions/9666/is-accessing-a-variable-in-c-sharp-an-atomic-operation – Ilian May 25 '12 at 06:37
  • @zmbq if there are several booleans that got packed into a single bitfield then setting is not atomic – ratchet freak May 25 '12 at 09:38
  • @AlexeiLevenkov 'Fields that are declared volatile are not subject to compiler optimizations that assume access by a single thread.' http://msdn.microsoft.com/ru-ru/library/x13ttww7.aspx – Dmitry Egorenkov May 25 '12 at 10:36
  • @UweKeim Atomicity is not needed in this case. – Dmitry Egorenkov May 25 '12 at 10:52
  • I was under the impression that using volatile in this scenario is not entirely safe. The reason (as I understand it) is that volatile only uses a half fence which means the write can get moved after the read. However I know that different processors handle this differently and multi-cpu systems can also do things differently which might explain why it works here. Also sticking the read in a loop probably helps. In real life it is normally preferable to use a lock. Finally in this instance it would be better still to use Thread.Join? – Martin Brown May 25 '12 at 11:56
  • @MartinBrown I am not sure that reordering applies here- because this is just a boolean, accesses are atomic as Ilian points out in the comments above. In the OP's case I believe the compiler was issuing an optimization that essentially assumed "once the variable is read, the value can safely assume to not have changed throughout the body of this method", resulting in memory access instructions executing in that hot loop being replaced by a processor cache or register access. All `volatile` does is "hey, someone might change that and I want to know about it", so the optimization isn't issued. – Chris Shain May 25 '12 at 14:38
  • @UweKeim as other have pointed out, atomicity is not an issue with a boolean (or any value type variable less than the word size of the native system, which generally includes ints, bytes, etc). Think of it this way- the minimum instruction data size that the CPU can use is (generally) 32 or 64 bits, so there is literally no way that a value of that size or smaller can be "split". – Chris Shain May 25 '12 at 14:41
  • @MartinBrown: True, but there is no case of a write-read sequence in either thread in play here. `volatile` is definitely sufficient in this case. Also, the read is already inside the loop even if it doesn't appear that way in C# at first glance. Finally, it is quite easy to reproduce a problem with `Thread.Join` as well. You see can my example [here](http://stackoverflow.com/a/6164855/158779). – Brian Gideon May 25 '12 at 18:36
  • @MartinBrown: By the way, I prefer `volatile` for trivial cases like these. `lock` is certainly fine, but the code looks ugly. Consider how `while (!stop) { }` has to change to be semantically equivalent to the `lock` approach. – Brian Gideon May 25 '12 at 18:51
  • @Brian/@Chris: Fair point about the reordering, but I wonder what the Thread.Sleep is going to be replaced with? Some care would have to be taken to ensure the correct result. – Martin Brown May 28 '12 at 14:42
  • @Brian: The example you reference is not having an issue with Thread.Join, it is having an issue with reading and setting a bool. My point was that in the nivlam's case you can do away with the bool altogether. This is preferable as looping around checking stop is effectively a spin-wait which will use up loads of CPU if it goes on too long. – Martin Brown May 28 '12 at 14:44
  • @MartinBrown: I'm totally with you on that. – Brian Gideon May 28 '12 at 23:37
  • This is really useful for me. Thanks for your nice answer!! – Mystic Lin Aug 18 '17 at 08:24
11

Because it's not thread safe you update the main thread variable stop inside the child thread. It will always be unpredictable. To work with any situation like this, have a look on this article.

The volatile keyword instructs the compiler to generate an acquire-fence on every read from that field, and a release-fence on every write to that field. An acquire-fence prevents other reads/writes from being moved before the fence; a release-fence prevents other reads/writes from being moved after the fence. These “half-fences” are faster than full fences because they give the runtime and hardware more scope for optimization.

ABH
  • 3,391
  • 23
  • 26
0

Thread Unsafe code is unpredictable. Main problem is changing one thread variable from another thread. Make the variable global or volatile. You can do it by following

static volatile bool stop = false;

static void Main(string[] args)
{    
    new Thread(() =>
    {
        Thread.Sleep(1000);
        stop = true;
        Console.WriteLine("Set \"stop\" to true.");

    }).Start();

    Console.WriteLine("Entering loop.");

    while (!stop)
    {
    }

    Console.WriteLine("Done.");
}
Md Kamruzzaman Sarker
  • 2,387
  • 3
  • 22
  • 38
0

Looks like some sort of optimization for value of local variable - changing to a field make it terminate ok (note that volatile or correct locking should be used in actual code):

using System;
using System.Threading;

class Program
{
    static bool stop = false;
    static void Main(string[] args)
    {

        new Thread(() =>
        {
            Thread.Sleep(1000);
            stop = true;
            Console.WriteLine("Set \"stop\" to true.");

        }).Start();

        Console.WriteLine("Entering loop.");

        while (!stop)
        {
        }

        Console.WriteLine("Done.");
    }
}
Alexei Levenkov
  • 98,904
  • 14
  • 127
  • 179