Re: [boost] [thread] Request review of new synchronisation object, boost::permit<>

Subject: Re: [boost] [thread] Request review of new synchronisation object, boost::permit<>
From: Peter Dimov (lists_at_[hidden])
Date: 2014-05-05 13:41:31

Niall Douglas wrote:

> I see no guarantee of such in either the Boost documentation or 
> http://www.cplusplus.com/reference/condition_variable/condition_variable/~condition_variable/. 
> You are safe going the other way round i.e. destroying a condvar which has 
> wait functions in the process of exiting in other threads.

POSIX says

"It shall be safe to destroy an initialized condition variable upon which no 
threads are currently blocked."

No threads are currently blocked upon the condition variable in your 
example. C++11 says the same:

"~condition_variable();
Requires: There shall be no thread blocked on *this."

It is true that the POSIX standard supplies an example in which the thread 
calling pthread_cond_broadcast destroys the condvar, and it's true that 
C++11 addresses this same case in the note, but I think that the normative 
text clearly applies to your case too.

> The way I came at it is this: examine this reduced code first:
>
> atomic<bool> &lock;
> int result=0;
> while(!cas_lock(lock));
> if(!result)  // can the compiler assume this is always false?
>
> Sure, the CAS lock prevents reordering of reads and writes either side of 
> the lock. But does it tell the compiler that it cannot assume that a 
> *local* variable's value will not have changed across the CAS lock?

The compiler is within its rights to assume here that 'result' doesn't 
change, because it can prove that its address is not available to another 
thread. This is not the case in your example. The address of 'result' is - 
quite obviously - available to another thread, so a compiler that could 
prove that it isn't would be wrong.

> Anyway, I still stick to my guns on that predicate checked values in wait 
> conditions really ought to be stored atomic to prevent reordering and 
> elision.

The classic mutex/cv pattern does provide the necessary guarantees. Using 
atomics makes it easier for the programmer to write incorrect code.

atomic<bool> pred;

Thread 1:

lock_guard<> lock( m );
while( !pred )
{
    cv.wait( lock );
}

Thread 2:

pred = true; // pred is atomic, needn't bother with m
cv.notify_one();

The above is wrong. The corrected version is either:

Thread 2:

{ lock_guard<> lock( m ); pred = true; }
cv.notify_one();

or (!)

Thread 2:

pred = true;
{ lock_guard<> lock( m ); }
cv.notify_one();