InterlockedIncrement vs EnterCriticalSection/counter++/LeaveCriticalSection

Question

I have some multithreaded code (see question Windows API Thread Pool simple example ) for which I am using a counter to identify a thread.

I have been advised to use an InterlockedIncrement to increment this counter in the thread's callback function. However this didn't seem to properly lock the variable, as I encountered some concurrency issues. I replaced the InterlockedIncrement by using a critical section manually : EnterCriticalSection/counter++/LeaveCriticalSection and this now works perfectly.

Why is it so ? Aren't the two options supposed to be strictly equivalent ? Note that I am talking about launching just a couple (about 10) of threads.

Answer 1

Your code is not using InterlockedIncrement correctly.

InterlockedIncrement(&(thread.threadCount)); 
DWORD tid = (thread.threadCount-1)%thread.size(); 

This performs an atomic increment of thread.threadCount, but instead of saving the atomically-incremented value, you ignore it and go back to the thread.threadCount variable (which may have been incremented by another thread in the meantime).

In your case, what happens is that two threads did an InterlockedIncrement nearly simultaneously, incrementing it from 1 to 2, then 2 to 3. Both threads then read thread.threadCount and got 3 back (then subtracted 1 to get a final result of 2).

The correct code is

LONG tidUnique = InterlockedIncrement(&(thread.threadCount));
DWORD tid = (tidUnique-1)%thread.size(); 

The unique incremented value is returned by InterlockedIncrement. You need to use that value in your computations if you want to see the unique value.