Re: [boost] [thread] RFC standard proposed mutex, read-write mutex, condition

From: Howard Hinnant (howard.hinnant_at_[hidden])
Date: 2007-08-21 14:34:15

On Aug 21, 2007, at 8:46 AM, Yuval Ronen wrote:

> Howard Hinnant wrote:
>> Here is a link to a reference implementation and a FAQ for mutexes,
>> locks and condition variables I am currently anticipating proposing
>> for C++ standardization (or subsequent TR).
>>
>> http://home.twcny.rr.com/hinnant/cpp_extensions/concurrency_rationale.html
>
> After some not-so-thorough reading of this, a few comments:
>
> 1. I couldn't understand what defer_lock is good for, even after  
> reading
> Q.9 of the FAQ. I believe the use-case shown in Q.9 should actually  
> use
> accept_ownership instead. Can you elaborate please?

See if this is any clearer:

http://home.twcny.rr.com/hinnant/cpp_extensions/concurrency_rationale.html#unique_lock_defer_lock

> 2. I disagree with the answer to Q.14. I think that shared_mutex  
> should
> be changed to something like:
>
> class shareable_mutex
> {
> public:
>     class exclusive_facade
>     {
>         shareable_mutex &__m;
>     public:
>         exclusive_facade(shareable_mutex &m) : __m(m) { }
>         void lock();
>         bool try_lock();
>         void unlock();
>     };
>
>     class shared_facade
>     {
>         shareable_mutex &__m;
>     public:
>         shared_facade(shareable_mutex &m) : __m(m) { }
>         void lock();
>         bool try_lock();
>         void unlock();
>     };
>
>     exclusive_facade &exclusive();
>     shared_facade &shared();
> };
>
> This way, the condition c'tor will not get the shareable_mutex, but
> rather the exclusive_facade or the shared_facade, which model Mutex.
> condition::wait() can then simply call Mutex::unlock(). An additional
> benefit (additional to having wait() not accept a lock) is that
> shared_lock can then be dropped, but I haven't really thought about  
> how
> well this idea works with upgrade_mutex/upgrade_lock.

This appears to me to be very nearly the "nested lock" design boost  
currently has, though one could certainly have both these nested  
facades *and* namespace-scope locks.

Either way, I believe this design wouldn't meet the use case which I  
didn't effectively communicate in #14:

Given a read-write mutex and its associated condition variable:

    my::shared_mutex rw_mut;
    std::condition<my::shared_mutex> cv(rw_mut);

client code wants to wait on that cv in two different ways:

1.  With rw_mut read-locked.
2.  With rw_mut write-locked.

If we initialized the condition variable with cv(rw_mut.exclusive()),  
then cv.wait() would wait with rw_mut write-locked, but we wouldn't be  
able to wait on cv with rw_mut read-locked.

If we initialized the condition variable with cv(rw_mut.shared()),  
then cv.wait() would wait with rw_mut read-locked, but we wouldn't be  
able to wait on cv with rw_mut write-locked.

This use case desires *both* types of waits on the *same* mutex/cv pair.

If we had both condition::wait() and template <class Lock>  
condition::wait(Lock&), then it could be confusing what was happening  
when one read:

    cv.wait();

The cv constructor might not be close to this call to wait.  It might  
even be in another translation unit.  Is that wait a read-wait or a  
write-wait?  One might have to search to find out.

<credit>
Fwiw, it was Peter Dimov who first proposed templating condition on  
Mutex, *and* the wait functions on Lock (N2178).  When I first saw  
that I had not sufficiently experimented with the shared_mutex/ 
condition combination and did not understand the motivation.
</credit>

> 3. A really minor thing - you suggest a mutex_debug class to ensure
> we're not using the platform's mutex-recursiveness. I strongly  
> recommend
> using this when developing on Windows, but there's a actually a  
> simpler
> way, IMO, to write it (without comparing thread ids):
>
> template <class Mutex>
> class non_recursive_assert_mutex
> {
>     Mutex mut_;
>     bool locked_;
>
> public:
>     non_recursive_assert_mutex() : mut_(), locked_(false) { }
>
>     void lock()
>     {
>         mut_.lock();
>         assert(!locked_);
>         locked_ = true;
>     }
>
>     bool try_lock()
>     {
>         if (mut_.try_lock())
>         {
>             assert(!locked_);
>             locked_ = true;
>             return true;
>         }
>         return false;
>     }
>
>     void unlock()
>     {
>         locked_ = false;
>         mut_.unlock();
>     }
> };

Thanks.  You're right, that is simpler and better.  I've changed the  
example to your code (if you don't mind).

-Howard