Search

Thursday, November 13, 2008

Is interlocked increment followed by comparison thread safe?

Party canceled after heavy downpour

Sorry about the blog title, my imagination failed me :(.

In our internal alias someone asked the question "Is the following thread safe"

if(Interlocked.Increment(ref someInt) == CONSTANT_VAL)
{
doSomeStuff();
}

My instant reaction was no because even though the increment is done in a thread safe way using System.Threading.Interlocked class, the comparison that follows is not safe.


My reasoning was that the "if" expression can be broken down to the following operations



  1. Fetch of someInt
  2. Increment operation
  3. Write back of someInt
  4. Comparison

The first 3 are done inside the Increment method and it provides concurrency protection and hence cannot be interleaved by another instruction.


So if two threads are running in parallel (one marked in red and the other in green) I assumed that the following interleaving is possible



  1. someInt is 4 and CONSTANT_VAL is 5
  2. Fetch of someint      -> someInt ==4
  3. Increment operation   -> someInt ==5
  4. Write back of someint -> someInt ==5
  5. Fetch of someint      -> someInt == 5
  6. Increment operation   -> someInt == 6
  7. Write back of someint -> someInt == 6
  8. comparison            -> compare 6 & CONSTANT_VAL
  9. comparison            -> compare 6 & CONSTANT_VAL

This means that the comparison of both thread will fail.


However, someone responded back that I was wrong as the return value is being used and not the written back value. This made me do some more investigation.


If I see the JITted code then it looks like

if (Interlocked.Increment(ref someInt) == CONSTANT_VAL)
00000024 lea ecx,ds:[002B9314h]
0000002a call 796F1221

0000002f mov esi,eax
00000031 cmp esi,5

The call (at 0x000002a) is to the native code inside CLR which in turn calls Win32 api (InterlockedIncrement).


However, the last 2 lines are the interesting ones. Register EAX contains the return value and comparison is happening against that and CONSTANT_VAL. So even if the second thread had already changed the value of someInt it doesn’t have any effect as the return of the first increment is being used and not the safeInt value in memory. So first comparison (step 8 above) will actually compare CONSTANT_VAL against 5 and succeed.

No comments: