Partager via

Is interlocked increment followed by comparison thread safe?

  • Article

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.

Comments

  • Anonymous
    November 14, 2008
    makes sense, because it is a function call, you're not comparing against the actual variable notice that nowhere the documentation mentions that the incremented value is returned when calling the function

  • Anonymous
    November 14, 2008
    The 'safety' is an artifact of the way the code is generated in this specific instance. I don't think a compiler need to maintain the 'safety', and having to check to compiler output all the time is probably not the most productive way to do multi-threaded computing.

  • Anonymous
    November 14, 2008
    The safety is not an artifact of the compiler, but a consequence of the fact that Interlocked.CompareExchange does an atomic compare-and-swap-return-original-value. The value is not read out of the location after the fact.

  • Anonymous
    November 14, 2008
    Eber: The function does mention that the incremented value is returned "Return Value Type: System.Int32 The incremented value" And yes this is not an artifact of the compiler. This post kind of outlines the steps I personally went through to figure out what was happening. In retrospect it was very obvious, but isn't everything is obvious in retrospect :)

  • Anonymous
    November 18, 2008
    The comment has been removed

  • Anonymous
    November 18, 2008
    MSDN's documentation is a bit weird.  The Win32 API always returned the correct result.  The compiler intrinsic has returned the correct result starting in Windows 98 and NT4, but in older Windows versions the compiler intrinsic could return something different from the new value of the object. How could the compiler intrinsic depend on the Windows version instead of depending on the compiler version?  And if the compiler intrinsic simply called the Windows API, then why was the compiler intrinsic less reliable than the Windows API in Windows 95 and NT 3.5?  Really weird. But anyway modern versions work, so .Net works too.

  • Anonymous
    November 18, 2008
    The comment has been removed

  • Anonymous
    August 14, 2009
    This post might enlighten you. http://www.eggheadcafe.com/conversation.aspx?messageid=34506793&threadid=34497502 The base idea is: if it is simply calling the native win32 API and you manage to run .net code on a 386 or older system, the result might surprise you

  • Anonymous
    June 09, 2010
    the register is per-thread, not thread shared, which means the thread1's eax is still 5, not 6.

  • Anonymous
    August 07, 2011
    Run this loop and explain the results please - Dim value As Integer Parallel.For(0, 1000, Sub(i) Debug.WriteLine(Interlocked.Increment(value).ToString)                                                      End Sub) I get - 3 4 5 6 7 1 2 10 11 12 13 14 8 9 17 18 etc etc...

  • Anonymous
    August 07, 2011
    At least it doesn't print the same value twice. So I guess the numbers are printed out of sequence because the threads are switching between the call to Interlocked.Increment and the actual output to the debug window...