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

From: Yuval Ronen (ronen_yuval_at_[hidden])
Date: 2007-08-21 15:22:48

Howard Hinnant wrote:
> 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

I'm afraid not...

This example has 3 lines, the first 2 create unique_lock with 
defer_lock, and the 3rd calls std::lock. Those unique_lock don't lock, 
because std::lock to lock. OK. But who unlocks? The unique_locks don't 
own the mutexes, and therefore don't unlock them. But someone needs to 
unlock, and it sounds logical that the unique_locks would... Had we used 
accept_ownership, the unique_locks would have owned the mutexes, and 
unlock them. That's the difference between defer_lock and 
accept_ownership, the ownership, isn't it?

>> 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.

Not quite like the "nested lock" design, IMO. Instead of requiring the 
shared_mutex have a lock/unlock/shared_lock/shared_unlock functions, as 
in your proposal, I require them to have two facades with lock/unlock 
functions. Equivalent, IMO.

> 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.

The last sentence starts with "This use case", but I see no use case. Do 
we really have such a use case? I haven't seen one yet. But even if we 
had, then maybe the solution is the same solution to the requirement you 
phrased as "The freedom to dynamically associate mutexes with condition
variables" or "The ability to wait on general mutex / lock types" 
(what's the difference between those two sentences anyway?) in your 
response to Peter. Add a 'set_mutex(mutex_type &)', or maybe even 
'set_mutex(mutex_type *)' to std::condition. I think it will solve this 
rare case.

>> 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).

I'm honored!
And along the lines of what you said to Kai, that "It is all to easy to 
read a proposal, agree with it, and
stay silent", I'd like to say that I certainly like the rest of your 
proposal :)