Re: [Boost-users] [boost.thread] destruction of condition variables

From: Howard Hinnant (hinnant_at_[hidden])
Date: 2008-07-28 13:49:13

On Jul 28, 2008, at 6:49 AM, Vladimir Pozdyaev wrote:

>  Anthony Williams:
>
>> "Peter Dimov" <pdimov_at_[hidden]> writes:
>>
>> > Anthony Williams:
>> >
>> >> However, it is never safe to destroy any object whilst another  
>> thread
>> >> might still be in a member function.
>> >
>> > I believe that this is a "POSIX destruction safety" issue. POSIX  
>> CVs
>> > are supposed to support such code.
>>
>> POSIX CVs are supposed to support broadcast immediately before
>> destroy. What we have here is the waiting thread doing the
>> destruction, which is not guaranteed safe by POSIX, as far as I know.
>
>
> Looking at, e.g., http://www.opengroup.org/onlinepubs/009695399/functions/pthread_cond_init.html 
>  reveals that "It shall be safe to destroy an initialized condition  
> variable upon which no threads are currently blocked." Am I right in  
> understanding that the wait-destroy sequence should be quite safe  
> this way (provided no one else waits)?

I believe your code as shown is subject to "spurious wakeup".  If the  
wait ends (spuriously) before the notify_all() completes, or is even  
initiated, then the notify happens on a destructed cv.  POSIX says  
that in this case the pthread_cond_broadcast will return EINVAL.

The sentence you refer to means that the thread doing the notifying  
can safely destroy the cv even if the waiting thread has not yet  
returned from the wait (as long as that thread or any other do not try  
to initiate a new wait after the final signal/broadcast).  Note though  
that it is not safe to destruct the mutex which is being waited on,  
until the waiting thread wakes from the wait (locking the mutex) and  
then unlocks that mutex.

I recommend:

#include <boost/thread.hpp>
#include <boost/bind.hpp>

class C {
private:
         boost::mutex m;
         boost::condition_variable cv;
         bool quit_;
public:
          C() : quit_(false) {}
         ~C() {
                 boost::mutex::scoped_lock l( m );
                 while (!quit_)
                    cv.wait( l );
         }
         void fn() {
                  
boost::this_thread::sleep( boost::posix_time::milliseconds( 500 ) );
                 boost::mutex::scoped_lock l( m );
                 quit_ = true;
                 cv.notify_all();
         }
};

int main() {
         C c;
         boost::thread t( boost::bind( &C::fn, &c ) );
         return 0;
}

Disclaimer: untested.

Analysis: A cv.wait() is not required to wait at all (spurious  
wakeups).  Now, even if a poor quality cv didn't wait, ~C() waits  
until fn() sets quit_ to true.  fn() locks m before touching quit_.   
This ensures proper memory visibility between the two threads.  Each  
thread is reading/writing the shared memory only under a locked m.   
Next fn() signals ~C() *while still holding m*.  This ensures that  
~C() doesn't spuriously wake and spuriously see the updated value of  
quit_ after quit_ has been updated by fn() but before fn() finishes  
referencing all member data (cv).  Only after fn() unlocks m on exit  
is ~C() allowed to wake and inspect quit_ (it must acquire m to  
wake).  On finding it true, ~C() can now safely unlock m, destruct cv  
and destruct m.

-Howard