Tim Schmelter Tim Schmelter - 19 days ago 6
C# Question

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

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

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.