Re: [Boost-users] Deadlock issue with boost::condition

From: Matt Schuckmann (matt_at_[hidden])
Date: 2006-02-02 12:27:14

Hi, actually I've discovered that isn't even right the work thread needs 
to lock the mutex before it sets the worker_thread_finished_working flag 
to true.
Plus given boost
bool worker_thread_finished = false;

Thread1() //Worker thread
{
    while(1)
    {
      DoSomeStuff();
      { scoped_lock l( mutex1 )
        worker_thread_finished = true;
        condition1.notify_all()
      }

      scoped_lock l(mutex2);
      condition2.wait(l);
    }
}

Thread2() //Master thread.
{
    while(1)
    {
      scoped_lock l(mutex1);
      while( !worker_thread_finished )
        condition1.wait(l)
      worker_thread_finished = false;

      DoSomeOtherStuff();

      condition2.notify_all();
    }
}

You can also change the boolean flag to a predicate and use the form of 
wait that takes a predicate and implements the while loop for you.

The key is the worker thread must hold mutex1 while it changes the state 
flag/predicate, otherwise notify could be called between the check in 
the while loop and call to wait and the deadlock would occur.

I'm kind of bothered that the documentation doesn't state this.

This whole issue make me think that this problem could be much more 
easily solved with with a binary semaphore or a non recursive mutex. 
Then it doesn't matter if the worker thread gets done before the master 
thread starts to wait.

Thanks
Matt S.

Vladimir Prus wrote:
> Hi Matt,
> 
>> To further complicate the matter thread1 can't call notify_all again
>> until it has been signaled to run (via a different condition variable)
>> which is signaled by thread2.
>>
>> Does this make sense?
>> Here's some pseudo code for what I'm doing.
> 
>> Thread2() //Master thread.
>> {
>>    while(1)
>>    {
>>      scoped_lock l(mutex1);
>>      condition1.wait(l)
> 
> This is wrong. Your code should be written like this:
> 
>   bool worker_thread_finished_working = false;
> 
>   ....
>   scoped_lock l(mutex1);
>   while(!worker_thread_finished_working)
>         condition1.wait(l);
> 
> 
> That way, if worker thread has finished working before you enter wait, you
> simply won't enter the loop and won't call condition1.wait. Call to
> 'notify_all' on condition should be considered as meaning "something in the
> world has changed", and after waking from 'wait' you should reevaluate
> necessary variables to decide if the wait is done or not. Using
> 'notify_all' in any other way is risky.
> 
> Some further notes can be found at:
> http://vladimir_prus.blogspot.com/2005/07/spurious-wakeups.html
> 
> - Volodya