[boost] [thread] Expected behaviour of condition variable wait when the mutex is not locked

From: vicente.botet (vicente.botet_at_[hidden])
Date: 2008-05-04 13:45:17

----- Original Message ----- 
From: "Anthony Williams" <anthony_at_[hidden]>
To: <threads-devel_at_[hidden]>
Sent: Monday, April 14, 2008 5:34 PM
Subject: Re: [Threads-devel] [boost] Expected behaviour of condition 
variable wait when unique_lock is not locked

> Quoting "vicente.botet" <vicente.botet_at_[hidden]>:
>
>> which is the expected behaviour of condition variable wait when
>> unique_lock is not locked
>
> Undefined behaviour: a precondition has not been met.

IMO this is not satisfactory. At least an exception should be thrown. And 
the C++0x recomendation states this.

"void wait(unique_lock<mutex>& lock);
Precondition:
    lock is locked by the current thread, and either:
    No other thread is waiting on this condition_variable object, or
    The lock arguments supplied by all concurrently waiting threads (via 
wait or timed_wait) return the same value for lock.mutex().
Effects:
    Atomically calls lock.unlock() and blocks on *this.
    When unblocked, calls lock.lock() (possibly blocking on the lock) and 
returns.
    The function will unblock when this thread is signaled by a call to 
this->notify_one(), a call to this->notify_all(), or spuriously.
    If the function exits via an exception, lock.lock() will still be called 
prior to exiting the function scope.
Postconditions:
    lock is locked by the current thread.
Throws:
    system_error when the effects or postconditions cannot be achieved.
"

I'm wondering why the condition_variable wait operation do not requires a 
strict lock (the one introduced by Andrei Alexandrescu in his article about 
external locking "Multithreading and the C++ Type System") instead of a 
unique lock or whatever. In this way the interface will force the 
precondition. Please let me know if this has already been discused in this 
list.

template <typename Lockable>
class strict_lock : private boost::noncopyable /*< Is not copyable >*/ {
    BOOST_CONCEPT_ASSERT((LockableConcept<Lockable>));
public:
    typedef Lockable lockable_type;
    explicit strict_lock(lockable_type& obj)
        : obj_(obj) { obj.lock(); } /*< locks on construction >*/
    ~strict_lock() { obj_.unlock(); } /*< unlocks on destruction >*/

    typedef bool (strict_lock::*bool_type)() const; /*< safe bool idiom >*/
    operator bool_type() const { return &strict_locker::owns_lock; }
    bool operator!() const { return false; } /*< always owned >*/
    bool owns_lock() const { return true; }
    const lockable_type* mutex() const { return &obj_; }

    bool is_locking(lockable_type* l) const { return l==mutex(); } /*< 
strict lockers specific function >*/
    /*< no possibility to unlock >*/
private:
    lockable_type& obj_;
    strict_lock(); /*< disable default constructor >*/
    BOOST_NON_ALIAS(strict_lock); /*< disable aliasing >*/
    BOOST_NON_HEAP_ALLOCATED(strict_lock); /*< disable heap allocation >*/
};

namespace boost {
class condition_variable
    // ...
    void wait(strict_lock<mutex>& l) {
#    ifndef BOOST_THREAD_CONDITION_VARIABLE_DONT_CHECK_SAME
        /*< define BOOST_THREAD_CONDITION_VARIABLE_DONT_CHECK_SAME
            if you don't want to check locker check the same lockable >*/
        if (!l.is_locking(&m)) throw lock_error(); /*< run time check throw 
if not locks the same >*/
#    endif
      // ... do as for unique_lock
    }
};
}

Evidently we can have more that one way to implement a strict lock but we 
have no way to check at compile time that a class is a model of a strict 
lock, so we need to relai on the "parolle" of the library author and add 
some optional run time checks.
The condition_variable interface can be extended to accept arbitrary locks 
satisfying some constraints.

    template <class Locker>
    void wait(Locker& l) {
        BOOST_CONCEPT_ASSERT((StrictLockerConcept<Locker>));
        BOOST_STATIC_ASSERT((is_strict_lock<Locker>::value));
        /*< Locker is a strict lock "sur parolle" >*/
        BOOST_STATIC_ASSERT((is_same<
            Lockable,
            typename lockable_type<Locker>::type>::value));
        /*< that locks the same lockable type >*/
#      ifndef BOOST_THREAD_CONDITION_VARIABLE_DONT_CHECK_OWNERSHIP
        /*< define BOOST_THREAD_CONDITION_VARIABLE_NO_CHECK_OWNERSHIP
            if you don't want to check locker ownership >*/
        if (! l) throw lock_error(); /*< run time check throw if no locked 
 >*/
#    endif
#    ifndef BOOST_THREAD_CONDITION_VARIABLE_DONT_CHECK_SAME
        /*< define BOOST_THREAD_CONDITION_VARIABLE_DONT_CHECK_SAME
            if you don't want to check locker check the same lockable >*/
        if (!l.is_locking(&m)) throw lock_error(); /*< run time check throw 
if not locks the same >*/
#    endif
      // ... do as for unique_lock
    }

is_strict_lock must be specialized by the strict locker implementer to state 
"sur parolle" that the class is a strict lock.

Any comments?

____________________
Vicente Juan Botet Escriba