Question

The naming of this function seems like this is some complicated stuff going on. When exactly does one know that this is the way to go instead of doing something like this:

Preparation CRITICAL_SECTION cs; int *p = malloc(sizeof(int)); // Allocation Site InitializeCriticalSection(&cs); // HINT for first Write

Thread #1 { *p = 1; // First Write }

Thread #2 { EnterCriticalSection(&cs); *p = 2; // Second Write LeaveCriticalSection(&cs); }

I have a write that gets done in one thread:

Run()
{
// some code
m_bIsTerminated = TRUE;
// some more code
}

Then, I have a read that gets done in another thread (potentially at the same time):

Terminate()
{
// some code
if( m_bIsTerminated )
{
m_dwThreadId = 0;
m_hThread = NULL;
m_evExit.SetEvent();
return;
}
// even more code
}

What's the best solution to solve this race condition? Are critical sections the way to go or is the use of InterlockedExchangeAdd() more useful?

Was it helpful?

Solution

InterlockedExchangeAdd is used to add a value to an integer as an atomic operation, meaning that you won't have to use a critical section. This also removes the risk of a deadlock if one of your threads throws an exception - you need to make sure that you don't keep any lock of any kind as that would prevent other threads from acquiring that lock.

For your scenario you can definitely use an Interlocked...- function, but I would use an event (CreateEvent, SetEvent, WaitForSingleObject), probably because I often find myself needing to wait for more than one object (you can wait for zero seconds in your scenario).

Upd: Using volatile for the variable may work, however it isn't recommended, see: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2016.html and http://www-949.ibm.com/software/rational/cafe/blogs/ccpp-parallel-multicore/tags/c%2B%2B0x for instance.

If you want to be portable, take a look at boost::thread.

OTHER TIPS

In your case, there's no race condition. The variable is never reset back to FALSE, is it? It's just a "please die" switch for the thread, right? Then no need for synchronization of any kind.

The InterlockedXXX family of functions makes use of Intel CPU's atomic 3-operand commands (XADD and CMPXCNG). So they're much cheaper than a critical section. And the one you want for thread-safe assignment is InterlockedCompareExchange().

UPD: and the mark the variable as volatile.

Make sure m_bIsTerminated is marked as volatile, and you should be ok. Although it seems pretty weird to me that you'd // some more code after setting "is terminated" to true. What exactly does that variable indicate?

Your "race condition" is that your various elements of // more code can execute in different orders. Your variable doesn't help that. Is your goal to get them to execute in a deterministic order? If yes, you'd need a condition variable to wait on one thread and set in another. If you just don't want them executing concurrently, a critical section would be fine.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top