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

From: Howard Hinnant (hinnant_at_[hidden])
Date: 2007-08-22 11:55:22

On Aug 21, 2007, at 6:31 PM, Peter Dimov wrote:

> Howard Hinnant wrote:
>
> ...
>
>> No, not ignoring your suggestion.  Still contemplating it.  Of all  
>> the
>> condition suggestions I've heard (not just here, but over the past
>> month or so) I like yours second best. ;-)
>>
>> I'm concerned that we would be relegating goal #3:
>>
>>> 1.  Minimum size - performance overhead.
>>> 2.  The freedom to dynamically associate mutexes with condition
>>> variables.
>>> 3.  The ability to apply run time check consistency concerning the
>>> associated mutex.
>>> 4.  The ability to wait on general mutex / lock types.
>>
>> to a non-standard-mandated debug library.  In your favor, I believe
>> most current vendors are supplying debug libs.  However the checks  
>> are
>> not uniform.  Even the error reporting mechanism isn't uniform.
>
> In general, I don't like designs that mandate checking by specifying
> behavior on error because they make incorrect programs correct. That  
> is, it
> is no longer possible for the library to loudly flag invalid uses as  
> the
> client may exploit the documented behavior and expect (or worse,  
> ignore) the
> exception. It also requires that the library catches 100% of the  
> errors,
> rather than 99.4% of them, and these last 0.6% can be expensive.
>
> There could be legitimate use cases that require the exception. If  
> forced, I
> would probably supply checked_condition to address this need, if it  
> proves
> justified. Or I might not, as checked_condition seems trivial to  
> write as a
> wrapper over condition (unless I'm missing something - I haven't  
> actually
> prototyped the code).

I've given this a lot of thought.  By relegating goal #3 (mutex  
consistency checking) to a debug build, we are also endangering goal  
#1.  Here's the scenario:

As Peter suggests, we make it undefined behavior to call wait  
referencing a mutex other than the one passed to the constructor.

Vendor A provides a std::lib just as we anticipate:  goals 1, 2 and 4  
are met by the release build, and goal 3 is met by the debug build.

Vendor B provides a std::lib, but not a debug build (std::lib vendors  
exist today that do that).  So clients can now not portably depend on  
goal #3.  They must write their own checked_condition, which isn't  
that hard (prototype enclosed below), but it is definitely harder than  
the checked/unchecked method I had proposed.  On debug builds they may  
or may not be paying for double-checking (depending on which platform  
they are on).

Vendor C, with the best of intentions, decides that the release build  
of the std::lib should contain this checking (std::lib vendors  
including debug checks in release builds should sound familiar to many  
of you).  Now we have goal #3, but have lost goal #1, and there is no  
wrapper we can use to get it back.

How do we prevent, or at least discourage, Vendor C from taking this  
path?

One answer would be to eliminate the condition constructor taking a  
mutex.  And drop all references to error checking or undefined  
behavior.  We completely chuck goal #3.  Writing your own  
checked_condition is the only way to get this functionality.

The next step I fear (after we've gotten rid the condition(mutex)  
constructor) is that someone will notice we're not using the template  
parameter Mutex any longer.  Why template condition at all?  It  
needlessly complicates the interface.  As soon as we take the template  
parameter off a condition, there goes goal #1 again:

class condition
{
     pthread_mutex_t internal_mut_;  // this could also be a  
pthread_mutex_t*, and allocate on heap when needed
     pthread_cond_t  cv_;
public:
     ...
};

Another answer would be to cling to goal #1 and chuck goal #4:

class condition
{
     pthread_cond_t  cv_;
public:
     ...
};

Now you can *only* wait on a std::condition with a std::mutex (or a  
lock referencing a std::mutex).  There is no error checking.  There is  
no generalizing to shared_mutex.  The good news is that a more general  
(templated) g_condition<Mutex> can be built on top of such a  
std::condition.  The bad news is that it is sufficiently error prone  
that Anthony, Peter, and myself have all made very subtle multithread  
mistakes in implementing the generalized condition.  This is not  
something that is trivial to reinvent.  The correct (as far as I know)  
code for the general condition is freely available:

    http://home.twcny.rr.com/hinnant/cpp_extensions/condition

The source isn't that long.  But I repeat, it is subtle.

So, those of you who were very much in favor of goal #1 (zero  
overhead), and/or goal #3 (run time consistency checking), this is  
your chance to make lots of noise.  These goals are in danger of being  
dropped.

Here's checked_condition:

template <class Mutex>
class checked_condition
{
public:
     typedef Mutex mutex_type;
private:
     std::condition<mutex_type> cv_;
     mutex_type* mut_;
public:

     checked_condition() : mut_(0) {}
     explicit checked_condition(mutex_type& m) : mut_(&m) {}

     void notify_one() {cv_.notify_one();}
     void notify_all() {cv_.notify_all();}

     template <class Lock>
         void wait(Lock& lock)
         {
             if (!lock.owns() || (mut_ != 0 && lock.mutex() != mut_))
                 throw std::runtime_error("Or whatever error handling  
policy you want");
              cv_.wait(lock);
         }

     template <class Lock, class Predicate>
         void wait(Lock& lock, Predicate pred)
         {
             if (!lock.owns() || (mut_ != 0 && lock.mutex() != mut_))
                 throw std::runtime_error("Or whatever error handling  
policy you want");
              cv_.wait(lock, std::move(pred));
         }

     template <class Lock>
         bool timed_wait(Lock& lock, const std::utc_time& abs_time)
         {
             if (!lock.owns() || (mut_ != 0 && lock.mutex() != mut_))
                 throw std::runtime_error("Or whatever error handling  
policy you want");
              cv_.timed_wait(lock, abs_time);
         }

     template <class Lock, class Predicate>
         bool timed_wait(Lock& lock, const std::utc_time& abs_time,  
Predicate pred)
         {
             if (!lock.owns() || (mut_ != 0 && lock.mutex() != mut_))
                 throw std::runtime_error("Or whatever error handling  
policy you want");
              cv_.timed_wait(lock, abs_time, std::move(pred));
         }

};

-Howard