Skip to content

Commit

Permalink
From David Fries, "Here is a fix for a deadlock seen under Windows us…
Browse files Browse the repository at this point in the history
…ing OpenThreads

Barrier operations.  The error is with atomic operations in the
win32 condition implementation.  The attached sample program will
reliably trigger with as few as three threads and a dual core system,
though sometimes it will take 65,000 iterations.

2.8.1 was the base for these changes

Win32ConditionPrivateData.h
Win32ConditionPrivateData::wait does two operations to decrement
waiters_ then read, when InterlockedDecrement decrements and returns
the value in one operation.  The two operations allows another thread
to also decrement with both getting 0 for an answer.

Win32ConditionPrivateData::broadcast is using waiters_ directly
instead of using the w value read earlier, if it was safe to use
waiters_ directly there would be no need for InterlockedGet or w.

overview of deadlock in barrier with three threads
one thread in broadcast, 2 threads in wait,
release semaphore 2, waits on waiters_done_
both threads wake, decrement waiters_, get 0 for w,
       <logic error here>
one calls set waiters_done_,
broadcast thread comes out of waiters_done_,
other thread calls waiters_done_, (which leaves waiters_done_ in the
signaled state)
       <sets the trap>
broadcast thread returns releases mutex, other threads get
mutex and also return,
next barrier, first two threads enter wait, one goes to broadcast, release
semaphore 2, skips waiters_done_ as it had been released last time
returns, processes, enters the barrier for the next barrier operation
and waits,
three threads are now in wait, two have the previous barrier phase,
one the current phase, there's one count left in the semaphore which a
thread gets, returns, enters the barrier as a waiter, sleeps, and the
deadlock is completed"

Merged from svn/trunk using:

svn merge -r 10456:10457 http://www.openscenegraph.org/svn/osg/OpenSceneGraph/trunk/src/OpenThreads/win32
  • Loading branch information
robertosfield committed Jul 13, 2009
1 parent 3c9dbfd commit aca5ac4
Showing 1 changed file with 3 additions and 5 deletions.
8 changes: 3 additions & 5 deletions src/OpenThreads/win32/Win32ConditionPrivateData.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class Win32ConditionPrivateData {
if (have_waiters)
{
// Wake up all the waiters.
ReleaseSemaphore(sema_.get(),waiters_,NULL);
ReleaseSemaphore(sema_.get(), w, NULL);

cooperativeWait(waiters_done_.get(), INFINITE);

Expand Down Expand Up @@ -112,8 +112,7 @@ class Win32ConditionPrivateData {
}
catch(...){
// thread is canceled in cooperative wait , do cleanup
InterlockedDecrement(&waiters_);
long w = InterlockedGet(&waiters_);
long w = InterlockedDecrement(&waiters_);
int last_waiter = was_broadcast_ && w == 0;

if (last_waiter) SetEvent(waiters_done_.get());
Expand All @@ -123,8 +122,7 @@ class Win32ConditionPrivateData {


// We're ready to return, so there's one less waiter.
InterlockedDecrement(&waiters_);
long w = InterlockedGet(&waiters_);
long w = InterlockedDecrement(&waiters_);
int last_waiter = was_broadcast_ && w == 0;

if (result != -1 && last_waiter)
Expand Down

0 comments on commit aca5ac4

Please sign in to comment.