Do i need to lock a bool when i'm reading/writing it in two consecutive statements?

Question

I wonder if this is thread-safe or (if not) how i can make it safe. It gets called from a timer:

private volatile bool _isSynchronizing;

private void SynchronizeSessionCache(object state = null)
{
    if (_isSynchronizing)
    {
        Log.Warn($"Aborted synchronization of SessionCache with SessionManager because we are already synchronizing. Interval is: {SynchronizationInterval}");
        return;
    }

     _isSynchronizing = true;

    bool lockWasTaken = false;
    try
    {
        // some code  that doesn't need a lock ...
        // ...

        // lock this part
        Monitor.Enter(_lockObject, ref lockWasTaken);
        // main code ...
    } 
    finally  // omitted catch
    {
        _isSynchronizing = false; 
        if(lockWasTaken)
            Monitor.Exit(_lockObject);
    }
}

My concern is that it could be possible that one thread checks _isSynchronizing at the beginning of his method and it's false at this time. Then another thread enters the body because it's not yet set to true by thread1. Is that possible even if _isSynchronizing is volatile? If so, what's the best way to make this method thread-safe?

If i understand it correctly volatile cannot prevent such race conditions but only ensures that the variable isn't cached, so all threads always read the current value.

Answer 1

To ensure the thread safety, you have to make the comparison and the assignment to a single atomic operation. Otherwise there is always a chance that another thread can pass the comparison while the assignment is being performed by the first thread. The volatile keyword doesn't help here, because it rather tells the compiler (and the runtime) that no optimization is allowed which could change the sequence of the read-write operations this variable is involved into.

(More about the volatile thing you can read in this great article by Eric Lippert.)

The simple (slightly slower one) solution would be to set up a critical section around the comparison and the assignment:

lock (_someLockObject)
{
    if (_isSynchronizing)
    {
         return;
    }

   _isSynchronizing = true;    
}

There is a faster solution however, but not for a bool variable. For an int, you could use the Interlocked.CompareExchange() method.

Let's pretend that int _isSynchronizing = 0 means false and _isSynchronizing = 1 means true.

Then you can use this statement:

if (Interlocked.CompareExchange(ref _isSynchronizing, 1, 0) == 1)
{
    // If the original val == 1, we're already synchronizing
    return;
}

This is slightly faster than using a Monitor, but there is no bool overload.