Does this c# class variable need to be volatile?

Question

In the below C# code, RunHelper() is only called from Run(). Since the compiler knows about it, will it optimize it in such a way that if(terminate) is going to be replaced by if(false)?

class Dummy
{
    private bool terminate;

    public Dummy() 
    {
        terminate = false;
    }

    public void KillDummy() // Called from another thread
    {
        terminate = true;
    }

    public void Run()
    {
        terminate = false;
        RunHelper();
    }
    private void RunHelper() //Only called from Run function
    {
        if(terminate)
        {
            Console.WriteLine("Exiting");
        }
    }
}

It depents. You can run it in release mode and use profiler to check, or look at IL code — Backs, Jun 22 '18 at 02:15
a short activity you can do, make a method say `GetBool()` which always returns a `false` , now try `terminate = GetBool();` and see in release (optimised code) is this method being called ? — Amit, Jun 22 '18 at 03:26
Yes, it will probably have to be volatile because you're looking at so little a window that optimization *will almost certainly happen*. In short, you want a call to `KillDummy` that occurs **right** after `terminate = false;` in `Run` but **before** `if (terminate)` in `RunHelper` to trigger the if-statement. That's a very small window, and with the size of `RunHelper` it will almost certainly be inlined into `Run`, in which case a caching of the non-volatile variable will almost certainly occur. — Lasse V. Karlsen, Jun 22 '18 at 07:04
Most of the time, I'd just jump straight to something *designed* with threading in mind, such as `CancellationToken` or `ManualResetEvent` and just know that it'll work rather than fiddling around and thinking about volatiles, unless or until there's a proven *performance penalty* that using standard built-ins are the root cause of and is having a noticeable effect. — Damien_The_Unbeliever, Jun 22 '18 at 07:07
Yes, the x86 jitter can do this. Volatile.Read/Write() is optimal and should always be preferred over *volatile*. And you should certainly favor CancellationToken, this is the kind of stuff to never mess with when trying to make threaded code reliable. You always want provably correct code since debugging it is impossible. — Hans Passant, Jun 22 '18 at 08:03

Theraot · Answer 1 · 2018-06-22T06:56:47.503

When compiling from C# to IL, the optimization removing the check from RunHelper will not happen.

However, the JIT (that compiles from IL to machine code) could take RunHelper and inline a copy inside of Run to void the call, and then the optimization you fear can heppen. No, reflection does not prevent this, because you will only see the IL with reflection, but not the machine code.

However, even if that optimization does not happen, you need to worry about thread visibility. If the system uses this code relatively often, it will keep terminate in cache, and - depending on the CPU architecture※ - other threads may not be aware of changes made to it. This is why you should use volatile (or similar solution).

※: On a computer with a single hardware thread (single core, and no hyper-threading or similar technology) there is no problem. If there are multiple hardware threads, they all have a cache, and there may or may not be shared caches. In those cases, you risk the changes to the value being invisible virtually forever.

There are some other caveats with volatile that - as far as I can tell - do not affect your code.

For instance, reordering does not only happen in the compiler. Reorders can also happen in the CPU, that is, the CPU may not execute the machine code strictly in the order it recieve them (see Out-of-order execution). The use volatile puts contraints on that reordering. However, writes could still be moved after reads (see: Threading in C# - PART 4: ADVANCED THREADING). You will be needing full memory barrier to fix it (Interlocked.MemoryBarrier() in .NET Core and Thread.MemoryBarrier() otherwise).

And, of course, there is the ABA problem. If you face it, you would need Interlocked to solve it, which also means you cannot use volatile and will have to only use Interlocked or volatile operations (Volatile.Read/Volatile.Write in .NET Core or Thread.VolatileRead/Thread.VolatileWrite otherwise) to work with the variable, forcing you to change from bool to int, because Interlocked does not work with bool.

Does this c# class variable need to be volatile?

1 Answers1