views:

1041

answers:

3

This question is NOT about race-conditions, atomicity, or why you should use locks in your code. I already know about those.

UPDATE: My question isn't "does weirdness with volatile memory exist" (i know it does), my question is "doesn't the .NET runtime abstract that away so you'll never see it".

See http://www.yoda.arachsys.com/csharp/threads/volatility.shtml and the first answer on http://stackoverflow.com/questions/434890/is-a-string-property-itself-threadsafe

(They're really the same article since one references the other.) One thread sets a bool and the other thread loops forever reading that bool -- those articles claim the reading thread might cache the old value and never read the new value, so therefore you need a lock (or use the volatile keyword). They claim the following code will potentially loop forever. Now I agree it's good practice to lock your variables, but I can't believe the .NET runtime would really ignore a memory value changing as the article claims. I understand their talk about volatile memory vs non-volatile memory, and I agree they have a valid point in non-managed code, but I can't believe the .NET runtime won't correctly abstract that away so that the following code does what you expect. The article even admits the code will "almost certainly" work (though not guaranteed), so I'm calling BS on the claim. Can anyone verify that it's true the following code won't always work? Is anyone able to get even one case (maybe you can't always reproduce it) where this fails?

class BackgroundTaskDemo
{
    private bool stopping = false;

    static void Main()
    {
        BackgroundTaskDemo demo = new BackgroundTaskDemo();
        new Thread(demo.DoWork).Start();
        Thread.Sleep(5000);
        demo.stopping = true;
    }

    static void DoWork()
    {
         while (!stopping)
         {
               // Do something here
         }
    }
}
+18  A: 

The point is: it might work, but it isn't guaranteed to work by the spec. What people are usually after is code that works for the right reasons, rather than working by a fluke combination of the compiler, the runtime and the JIT, which might change between framework versions, the physical CPU, the platform, and things like x86 vs x64.

Understanding the memory model is a very very complex area, and I don't claim to be an expert; but people who are real experts in this area assure me that the behaviour you are seeing is not guaranteed.

You can post as many working examples as you like, but unfortunately that doesn't prove much other than "it usually works". It certainly doesn't prove that it is guaranteed to work. It would only take a single counter-example to disprove, but finding it is the problem...

No, I don't have one to hand.


Update with repeatable counter-example:

using System.Threading;
using System;
static class BackgroundTaskDemo
{
    // make this volatile to fix it
    private static bool stopping = false;

    static void Main()
    {
        new Thread(DoWork).Start();
        Thread.Sleep(5000);
        stopping = true;


        Console.WriteLine("Main exit");
        Console.ReadLine();
    }

    static void DoWork()
    {
        int i = 0;
        while (!stopping)
        {
            i++;
        }

        Console.WriteLine("DoWork exit " + i);
    }
}

Output:

Main exit

but still running, at full CPU; note that stopping has been set to true by this point. The ReadLine is so that the process doesn't terminate. The optimization seems to be dependent on the size of the code inside the loop (hence i++). It only works in "release" mode obviously. Add volatile and it all works fine.

Marc Gravell
Good answer. I wonder - is it not guaranteed on future hardware, or on present-day hardware? Seems silly to me to worry about something that may not work on future hardware.
Kent Boogaart
To clarify - is there a real-world combination of compiler, runtime and JIT that can be shown to break with this code?
Kent Boogaart
Yes! Posted; run it in release mode etc.
Marc Gravell
Great - thanks again.
Kent Boogaart
Wow. This is crazy. Lots of bugs in my code because of this! Cheers!
TheSoftwareJedi
Reproduced successfully. Must use x86 machine runtime, release mode, no debugging. With an x64 architecture this isn't optimized.
TheSoftwareJedi
@TheSoftwareJedi - What isnt optimized for x64?
Jon
+2  A: 

FWIW:

  • I have seen this compiler optimization from the MS C++ compiler (unmanaged code).
  • I don't know whether it happens in C#
  • It won't happen while debugging (compiler optimizations are automatically disabled while debugging)
  • Even if that optimization doesn't happen now, you're betting that they'll never introduce that optimization in future versions of the JIT compiler.
ChrisW
+1  A: 

This example includes the native x86 code as comments to demonstrate that the controlling variable ('stopLooping') is cached.

Change 'stopLooping' to volatile to "fix" it.

This was built with Visual Studio 2008 as a Release build and run without debugging.

 using System;

 using System.Threading;

/* A simple console application which demonstrates the need for 
 the volatile keyword and shows the native x86 (JITed) code.*/

static class LoopForeverIfWeLoopOnce
{

    private static bool stopLooping = false;

    static void Main()
    {
        new Thread(Loop).Start();
        Thread.Sleep(1000);
        stopLooping = true;
        Console.Write("Main() is waiting for Enter to be pressed...");
        Console.ReadLine();
        Console.WriteLine("Main() is returning.");
    }

    static void Loop()
    {
        /*
         * Stack frame setup (Native x86 code):
         *  00000000  push        ebp  
         *  00000001  mov         ebp,esp 
         *  00000003  push        edi  
         *  00000004  push        esi  
         */

        int i = 0;
        /*
         * Initialize 'i' to zero ('i' is in register edi)
         *  00000005  xor         edi,edi 
         */

        while (!stopLooping)
            /*
             * Load 'stopLooping' into eax, test and skip loop if != 0
             *  00000007  movzx       eax,byte ptr ds:[001E2FE0h] 
             *  0000000e  test        eax,eax 
             *  00000010  jne         00000017 
             */
        {
            i++;
            /*
             * Increment 'i'
             *  00000012  inc         edi  
             */

            /*
             * Test the cached value of 'stopped' still in
             * register eax and do it again if it's still
             * zero (false), which it is if we get here:
             *  00000013  test        eax,eax 
             *  00000015  je          00000012 
             */
        }

        Console.WriteLine("i={0}", i);
    }
}
Joe Erickson
Reproduced successfully. Must use x86 machine runtime, release mode, no debugging. With an x64 architecture this isn't optimized.
TheSoftwareJedi