Thread synchronization. Why exactly this lock isn't enough to synchronize threads [duplicate]
lock(ms_Lock) { }
this is meaningless construct. lock
gurantees exclusive execution of code inside it.
Let me explain why this empty lock
decreases (but doesn't eliminate!) the chance of data corruption. Let's simplify threading model a bit:
- Thread executes one line of code at time slice.
- Thread scheduling is done in strict round robin manner (A-B-A-B).
- Monitor.Enter/Exit takes significantly longer to execute than arithmetics. (Let's say 3 times longer. I stuffed code with
Nop
s that mean that previous line is still executing.) - Real
+=
takes 3 steps. I broke them down to atomic ones.
At left column shown which line is executed at the time slice of threads (A and B).At right column - the program (according to my model).
A B 1 1 SomeOperation(); 1 2 SomeOperation();2 3 Monitor.Enter(ms_Lock); 2 4 Nop();3 5 Nop();4 6 Monitor.Exit(ms_Lock);5 7 Nop();7 8 Nop();8 9 int temp = ms_Sum; 3 10 temp++;9 11 ms_Sum = temp; 4 10 5 11 A B 1 1 SomeOperation(); 1 2 SomeOperation();2 3 int temp = ms_Sum; 2 4 temp++;3 5 ms_Sum = temp; 3 4 4 5 5
As you see in first scenario thread B just can't catch thread A and A has enough time to finish execution of ms_Sum += 1;
. In second scenario the ms_Sum += 1;
is interleaved and causes constant data corruption. In reality thread scheduling is stochastic, but it means that thread A has more change to finish increment before another thread gets there.
This is a very tight loop with not much going on inside it, so ms_Sum += 1
has a reasonable chance of being executed in "just the wrong moment" by the parallel threads.
Why would you ever write a code like this in practice?
Why not:
lock(ms_Lock) { ms_Sum += 1;}
or just:
Interlocked.Increment(ms_Sum);
?
-- EDIT ---
Some comments on why would you see the error despite memory barrier aspect of the lock... Imagine the following scenario:
- Thread A enters the
lock
, leaves thelock
and then is pre-empted by the OS scheduler. - Thread B enters and leaves the
lock
(possibly once, possibly more than once, possibly millions of times). - At that point the thread A is scheduled again.
- Both A and B hit the
ms_Sum += 1
at the same time, resulting in some increments being lost (because increment = load + add + store).
As noted the statement
lock(ms_Lock) {}
will cause a full memory barrier. In short this means the value of ms_Sum
will be flushed between all caches and updated ("visible") among all threads.
However, ms_Sum += 1
is still not atomic as it is just short-hand for ms_Sum = ms_Sum + 1
: a read, an operation, and an assignment. It is in this construct that there is still a race condition -- the count of ms_Sum
could be slightly lower than expected. I would also expect that the difference to be more without the memory barrier.
Here is a hypothetical situation of why it might be lower (A and B represent threads and a and b represent thread-local registers):
A: read ms_Sum -> aB: read ms_Sum -> bA: write a + 1 -> ms_SumB: write b + 1 -> ms_Sum // change from A "discarded"
This depends on a very particular order of interleaving and is dependent upon factors such as thread execution granularity and relative time spent in said non-atomic region. I suspect the lock
itself will reduce (but not eliminate) the chance of the interleave above because each thread must wait-in-turn to get through it. The relative time spent in the lock itself to the increment may also play a factor.
Happy coding.
As others have noted, use the critical region established by the lock or one of the atomic increments provided to make it truly thread-safe.