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

View: New views
18 Messages — Rating Filter:   Alert me  

Parent Message unknown [thread] Expected behaviour of condition variable wait when the mutex is not locked

by Vicente Botet Escriba :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

----- Original Message -----
From: "Anthony Williams" <anthony@...>
To: <threads-devel@...>
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@...>:
>
>> 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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

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

by Johan Torp :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

I'm not sure what's best in this case. I find that there is often a certain trade-off between ease of use and runtime insecurities.

For instance, it would have been useful to have a strict shared_ptr which could never be null. It's implemented easily by removing the default constructor and reset() and check+throw for non-null ptrs in the regular constructor and reset(T*). You would lose default-constructibility so you probably want to make the decision optional. Even if this could be implemented nicely by some policy parameter+default parameter, the option would mean a larger and more complicated abstraction for the user.

Best Regards, Johan



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

by Vicente Botet Escriba :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

From: "Johan Torp" <johan.torp@...>
Sent: Tuesday, May 06, 2008 3:07 PM
Subject: Re: [boost] [thread] Expected behaviour of condition variable wait
when the mutex is not locked


>
> I'm not sure what's best in this case. I find that there is often a
> certain
> trade-off between ease of use and runtime insecurities.

Yes you are right, we need to maintain a ease of use. Could you show me why
my proposal using a strict_lock is  not easy to use?

Anyway I prefer (as most of you) to spend time solving compile time problems
before my application is delivered than debuging on the field runtime code
and look that the problem could be solved by the compiler.
If a condition_variable must have its mutex locked, why not provide an
interface that can 'ensure' that at compile time, if we can do it?
If different mutexes are supplied for concurrent wait() operations on the
same condition variable means undefined behaviour, why not check it.

I don't understand why we can not wait on a condition_variable once we have
a lock_guard (which is more strict than the unique_lock). But I'm surely
missing something. And why not to add another wait operation which is safe,
or in the worst case more safe than the current ones? Maybe some of the C++
experts can clarify all these questions.

In any case, we can always define a strict_condition class based on the
boost::condition_variable class having a strict lock as parameter of the
wait operation.

void strict_condition::wait(strict_lock<mutex>& lock) {
    // add here all the needed/wanted runtime checks
    unique_lock<mutex> ulock(lock); // strict_lock must define a conversion
operator without locking
    cnd_.wait(ulock); //cnd_ is a boos::condition_variable
    ulock.release(); // the mutex should not be unlocked
}

Even if we can do this wrapping I don't understand why we need to waste in
CPU cycles when we can do without.

BTW, I think that you should preserv the context on your next posts, it
would be easier for the others to follow.

Best regrads
_____________________
Vicente Juan Botet Escriba


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

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

by Johan Torp :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

vicente.botet wrote:
> I'm not sure what's best in this case. I find that there is often a
> certain
> trade-off between ease of use and runtime insecurities.

Yes you are right, we need to maintain a ease of use. Could you show me why
my proposal using a strict_lock is  not easy to use?
Don't get me wrong, strict_lock is easier to use than unique_lock - it has a smaller interface. Just as you do, I think the gained compile time security is worth quite a lot. However, if both classes are needed, the threading library gets bigger and more complicated and hence we have a trade-off. That was my point.

I guess condition_variable::wait() doesn't check if the lock is locked for performance reasons. I think that is a common strategy for most of the standard library.

I don't know the rationale why unique_lock has a default constructor and lock/unlock methods but I'm interested in hearing it. Anyone?


Best Regards, Johan

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

by Anthony Williams-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

"vicente.botet" <vicente.botet@...> writes:

> ----- Original Message -----
> From: "Anthony Williams" <anthony@...>
> To: <threads-devel@...>
> 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@...>:
>>
>>> 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.
> "

Then you're reading it differently to how it was written. The precondition is
"lock is locked by the current thread". If the precondition is violated, all
bets are off.

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

The wait must unlock the mutex and then lock it again. By using unique_lock
(which supports that), this possibility is explicit. condition_variable_any
will support any lock type that has unlock() and lock() operations.

Boost 1.34 and prior used hidden traits types to unlock and relock the mutex,
which I don't think is desirable.

The Boost 1.35 "strict lock" is boost::lock_guard.

Anthony
--
Anthony Williams            | Just Software Solutions Ltd
Custom Software Development | http://www.justsoftwaresolutions.co.uk
Registered in England, Company Number 5478976.
Registered Office: 15 Carrallack Mews, St Just, Cornwall, TR19 7UL

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

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

by Anthony Williams-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Johan Torp <johan.torp@...> writes:

> vicente.botet wrote:
>>
>>> I'm not sure what's best in this case. I find that there is often a
>>> certain
>>> trade-off between ease of use and runtime insecurities.
>>
>> Yes you are right, we need to maintain a ease of use. Could you show me
>> why
>> my proposal using a strict_lock is  not easy to use?
>>
>
> Don't get me wrong, strict_lock is easier to use than unique_lock - it has a
> smaller interface. Just as you do, I think the gained compile time security
> is worth quite a lot. However, if both classes are needed, the threading
> library gets bigger and more complicated and hence we have a trade-off. That
> was my point.

The library does provide both. unique_lock is flexible, lock_guard is strict.

> I guess condition_variable::wait() doesn't check if the lock is locked for
> performance reasons. I think that is a common strategy for most of the
> standard library.

Yes.

> I don't know the rationale why unique_lock has a default constructor and
> lock/unlock methods but I'm interested in hearing it. Anyone?

There are use cases where it is desirable to unlock early if certain
conditions are met, or defer locking until we need to. Also, sometimes it is
necessary to transfer lock ownership between scopes.

To this end, unique_lock is default-constructible and movable, and supports
lock and unlock operations.

If you don't need these facilities, use lock_guard: it locks on construction,
and unlocks on destruction with no flexibility whatsoever.

Anthony
--
Anthony Williams            | Just Software Solutions Ltd
Custom Software Development | http://www.justsoftwaresolutions.co.uk
Registered in England, Company Number 5478976.
Registered Office: 15 Carrallack Mews, St Just, Cornwall, TR19 7UL

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Re: [thread] Expected behaviour of condition variable waitwhen the mutex is not locked

by Vicente Botet Escriba :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

----- Original Message -----
From: "Anthony Williams" <anthony_w.geo@...>
To: <boost@...>
Sent: Thursday, May 08, 2008 10:47 AM
Subject: Re: [boost] [thread] Expected behaviour of condition variable
waitwhen the mutex is not locked


> "vicente.botet" <vicente.botet@...> writes:
>
>> ----- Original Message -----
>> From: "Anthony Williams" <anthony@...>
>> To: <threads-devel@...>
>> 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@...>:
>>>
>>>> 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.
>> "
>
> Then you're reading it differently to how it was written. The precondition
> is
> "lock is locked by the current thread". If the precondition is violated,
> all
> bets are off.

You are right, as usual. It's the reference implementation described in
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2406.html#condition_rationale 
which induced me on error.

>From
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2406.html#condition_rationale
"Below is an example implementation of cond_var on top of pthread_cond_t.
The reference implementation is meant to demonstrate how thinly cond_var
maps to pthread_cond_t (or whatever native OS condition variable is
available).
class cond_var
{
    pthread_cond_t cv_;
public:

    cond_var()
    {
        error_code::value_type ec = pthread_cond_init(&cv_, 0);
        if (ec)
            throw system_error(ec, native_category, "cond_var constructor
failed");
    }

<snip>
    void wait(unique_lock<mutex>& lock)
    {
        error_code::value_type ec = pthread_cond_wait(&cv_,
lock.mutex()->native_handle());
        if (ec)
            throw system_error(ec, native_category, "cond_var wait failed");
    }
"

What do you think to add the raison you have prefered to abort the program
instead of throwing an exception on the rationale of your documentation?

The following wait implementation works with models of strict locks. The
user is able to add the runtime checks adding the corresponding defines.
This implementation do not suffer from undefined behaviour when:
* Different mutexes were supplied for concurrent wait() or timedwait()
operations on the same condition variable.
* The mutex was not owned by the current thread at the time of the call.

But
* needs to store the reference to the mutex on the condition_variable. When
the user do not defines them the behaviour is equivalent.
* Strict Lock must provide in addition
    - owns_lock
    - is_locking, and
    - specialize is_strict_lock

What is wrong on this approach?


    template <class Locker>
    void wait(Locker& l) {
        BOOST_CONCEPT_ASSERT((StrictLockConcept<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
    }

>> 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.
>
> The wait must unlock the mutex and then lock it again. By using
> unique_lock
> (which supports that), this possibility is explicit.

>From the Thread documentation:
"
Effects:
Atomically call lock.unlock() and blocks the current thread. The thread will
unblock when notified by a call to this->notify_one() or this->notify_all(),
or spuriously. When the thread is unblocked (for whatever reason), the lock
is reacquired by invoking lock.lock() before the call to wait returns. The
lock is also reacquired by invoking lock.lock() if the function exits with
an exception.
"

Here it is your implementation

inline void condition_variable::wait(unique_lock<mutex>& m)
{
    detail::interruption_checker check_for_interruption(&cond);
    BOOST_VERIFY(!pthread_cond_wait(&cond,m.mutex()->native_handle()));
}

I don't see any call to m.unlock()/m.lock(). Should you change the Effects
section?
Why we can not have:

inline void condition_variable::wait(strict_lock<mutex>& m)
{
    detail::interruption_checker check_for_interruption(&cond);
    BOOST_VERIFY(!pthread_cond_wait(&cond,m.mutex()->native_handle()));
}

> condition_variable_any
> will support any lock type that has unlock() and lock() operations.

condition_variable_any is another history. From
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2406.html#condition_rationale

"
I have experimented with templating the condition variable but have
discovered problems with this approach.
* If the condition is templated on lock type, then the wait functions are
not templated. This destroys the ability to simultaneously wait on a
unique_lock<shared_mutex> and a shared_lock<shared_mutex> on the same
shared_mutex.
* If the condition is templated on mutex type, then the wait functions can
be templated on lock type, solving the previous problem. However one is
still depending on a specialization of this condition to provide the razor
thin layer over the OS condition variable (e.g. pthread_cond_t). That
specialization can not reliably have its wait functions templated on lock
type. Such a lock would be required to do nothing but lock/unlock the mutex,
which would outlaw user defined lock types such as the locked file example
mentioned in the mutex rationale. The specialization must only allow waiting
on a standard lock type (i.e. unique_lock<mutex>).

Because the condition specialized on the native mutex type can not have the
same interface as the primary condition template (it can't wait on any lock
type), specialization is not appropriate for this application (reference the
vector<bool> example).

The only conclusion I can come to which supports both a razor thin layer
over the native OS condition variable, and a generalized condition variable
which works with user defined mutexes/locks (such as the Lock2 example) is
two distinct types:
* cond_var: A condition variable that can wait on nothing but
unique_lock<mutex> (or perhaps mutex).
* gen_cond_var: A condition variable that can wait on anything which
supports lock() and unlock().
"

I'm not asking to have a single condition_variable, neither asking to add
    void condition_variable_any::wait(strict_lock<mutex>& m)

We can have a strict_lock which define this lock/unlock operations private
and declares friend condition_variable_any.

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; }

    bool is_locking(lockable_type* l) const { return l==mutex(); } /*<
strict lockers specific function >*/
    /*< no possibility to unlock other than with conditional variables>*/
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 >*/

    friend class boost::condition_variable;
    friend class boost::condition_variable_any;
    lockable_type* mutex() { return &obj_; }
    void lock() {obj_.lock();}
    void unlock() {obj_.unlock();}

};

Evidently this strict_lock will not be locked while waiting on a condition
variable and the operation own_lock will be erroneus. But this operation can
not be called because the thread is waiting on the condition. Note that the
current implementation of condition_variable::wait(unique_lock<mutex>& m)
produce the same symptomes on the unique_lock.

>
> Boost 1.34 and prior used hidden traits types to unlock and relock the
> mutex,
> which I don't think is desirable.
>
> The Boost 1.35 "strict lock" is boost::lock_guard.
The raison we can not use a lock_guard is that it not provides the mutex
operation. It will be enough that lock_guard declares mutex as private
operation and make condition_variable a friend class.

Next follows some additions to lock_guard that without changing the public
interface, allows to use it with a condition_variable_any.

template<typename Mutex>
class lock_guard
{
private:
    Mutex& m;
    explicit lock_guard(lock_guard&);
    lock_guard& operator=(lock_guard&);
    // new
    friend class boost::condition_variable_any;
    friend class boost::condition_variable;
    Mutex* mutex() { return &m; }
    void lock() {m.lock();}
    void unlock() {m.unlock();}

public:
    explicit lock_guard(Mutex& m_):
    m(m_)
    {
        m.lock();
    }
    lock_guard(Mutex& m_,adopt_lock_t):
        m(m_)
    {}
    ~lock_guard()
    {
        m.unlock();
    }
    // new
    bool is_locking(Mutex* l) const
    {
        return l==mutex(); }
    };

With this lock_guard, is there any deep raison to don't have:

void condition_variable::wait(lock_guard<mutex>& m);

Best

Vicente


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Re: [thread] Expected behaviour of condition variable waitwhen the mutex is not locked

by Anthony Williams-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

"vicente.botet" <vicente.botet@...> writes:

> From: "Anthony Williams" <anthony_w.geo@...>

>> "vicente.botet" <vicente.botet@...> writes:
>>
>>> From: "Anthony Williams" <anthony@...>
>>>> Quoting "vicente.botet" <vicente.botet@...>:
>>>>
>>>>> 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.

>> Then you're reading it differently to how it was written. The precondition
>> is
>> "lock is locked by the current thread". If the precondition is violated,
>> all
>> bets are off.
>
> You are right, as usual. It's the reference implementation described in
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2406.html#condition_rationale 
> which induced me on error.
>
> What do you think to add the raison you have prefered to abort the program
> instead of throwing an exception on the rationale of your documentation?

I think it's a logic error in the application, so should be detected as soon
as possible during testing. You can always change it to throw by using the
"throw on assert" option for Boost.Assert.

> The following wait implementation works with models of strict locks. The
> user is able to add the runtime checks adding the corresponding defines.
> This implementation do not suffer from undefined behaviour when:
> * Different mutexes were supplied for concurrent wait() or timedwait()
> operations on the same condition variable.
> * The mutex was not owned by the current thread at the time of the call.
>
> But
> * needs to store the reference to the mutex on the condition_variable. When
> the user do not defines them the behaviour is equivalent.
> * Strict Lock must provide in addition
>     - owns_lock
>     - is_locking, and
>     - specialize is_strict_lock
>
> What is wrong on this approach?
>
>
>     template <class Locker>
>     void wait(Locker& l) {
>         BOOST_CONCEPT_ASSERT((StrictLockConcept<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
>     }

You could do those checks with unique_lock.

>>> 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.
>>
>> The wait must unlock the mutex and then lock it again. By using
>> unique_lock
>> (which supports that), this possibility is explicit.
>
>>From the Thread documentation:
> "
> Effects:
> Atomically call lock.unlock() and blocks the current thread. The thread will
> unblock when notified by a call to this->notify_one() or this->notify_all(),
> or spuriously. When the thread is unblocked (for whatever reason), the lock
> is reacquired by invoking lock.lock() before the call to wait returns. The
> lock is also reacquired by invoking lock.lock() if the function exits with
> an exception.
> "
>
> Here it is your implementation
>
> inline void condition_variable::wait(unique_lock<mutex>& m)
> {
>     detail::interruption_checker check_for_interruption(&cond);
>     BOOST_VERIFY(!pthread_cond_wait(&cond,m.mutex()->native_handle()));
> }
>
> I don't see any call to m.unlock()/m.lock(). Should you change the Effects
> section?

No. That's the pthread version, where the OS does the unlock/lock. The Windows
version has to do that explicitly. Since this version only takes
unique_lock<mutex>, it can rely on knowing what the implementation of
unique_lock<mutex> looks like for optimization (as in this implementation).

> Why we can not have:
>
> inline void condition_variable::wait(strict_lock<mutex>& m)
> {
>     detail::interruption_checker check_for_interruption(&cond);
>     BOOST_VERIFY(!pthread_cond_wait(&cond,m.mutex()->native_handle()));
> }

If your strict lock exposes the mutex with a .mutex() member function, you
can't guarantee strictness.

> I'm not asking to have a single condition_variable, neither asking to add
>     void condition_variable_any::wait(strict_lock<mutex>& m)
>
> We can have a strict_lock which define this lock/unlock operations private
> and declares friend condition_variable_any.

Yes, you could.

> Evidently this strict_lock will not be locked while waiting on a condition
> variable and the operation own_lock will be erroneus. But this operation can
> not be called because the thread is waiting on the condition. Note that the
> current implementation of condition_variable::wait(unique_lock<mutex>& m)
> produce the same symptomes on the unique_lock.

True.

>>
>> Boost 1.34 and prior used hidden traits types to unlock and relock the
>> mutex,
>> which I don't think is desirable.
>>
>> The Boost 1.35 "strict lock" is boost::lock_guard.
> The raison we can not use a lock_guard is that it not provides the mutex
> operation. It will be enough that lock_guard declares mutex as private
> operation and make condition_variable a friend class.
>
> Next follows some additions to lock_guard that without changing the public
> interface, allows to use it with a condition_variable_any.

> With this lock_guard, is there any deep raison to don't have:
>
> void condition_variable::wait(lock_guard<mutex>& m);

I guess not. I'll think about it.

Anthony
--
Anthony Williams            | Just Software Solutions Ltd
Custom Software Development | http://www.justsoftwaresolutions.co.uk
Registered in England, Company Number 5478976.
Registered Office: 15 Carrallack Mews, St Just, Cornwall, TR19 7UL

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

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

by Frank Mori Hess :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Thursday 08 May 2008 04:51 am, Anthony Williams wrote:
> To this end, unique_lock is default-constructible and movable, and supports
> lock and unlock operations.

Since when is unique_lock default-constructible?  You just mean it has a
defer_lock_t constructor, right?

- --
Frank
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFIIwYZ5vihyNWuA4URAhxKAJ42W5QM1BOKvMIXslK59Enohu+QagCeLVVR
PMkg5+0sUrPgYXF5TOajC2w=
=RjpP
-----END PGP SIGNATURE-----
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Re: [thread] Expected behaviour of condition variable waitwhen the mutex is not locked

by Peter Dimov-5 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Frank Mori Hess:
> On Thursday 08 May 2008 04:51 am, Anthony Williams wrote:
>> To this end, unique_lock is default-constructible and movable, and
>> supports
>> lock and unlock operations.
>
> Since when is unique_lock default-constructible?  You just mean it has a
> defer_lock_t constructor, right?

It does have a default constructor. Movable types have to have an "empty"
state in which to put the moved-from source, and the default constructor of
a movable type usually creates such an "empty" object. (The proposed promise
doesn't adhere to this "convention" though.)

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

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

by Anthony Williams-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Frank Mori Hess <frank.hess@...> writes:

> On Thursday 08 May 2008 04:51 am, Anthony Williams wrote:
>> To this end, unique_lock is default-constructible and movable, and supports
>> lock and unlock operations.
>
> Since when is unique_lock default-constructible?  You just mean it has a
> defer_lock_t constructor, right?

Yes, that's what I meant: you can construct it without locking a mutex. I got
confused by Johan's statement.

Actually, thinking about it, there's no reason not to give it a default
constructor: you'd just not be able to do anything with it until you
move-assigned a unique_lock with a mutex into it.

Anthony
--
Anthony Williams            | Just Software Solutions Ltd
Custom Software Development | http://www.justsoftwaresolutions.co.uk
Registered in England, Company Number 5478976.
Registered Office: 15 Carrallack Mews, St Just, Cornwall, TR19 7UL

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Re: [thread] Expected behaviour of condition variablewaitwhen the mutex is not locked

by Vicente Botet Escriba :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


----- Original Message -----
From: "Peter Dimov" <pdimov@...>
To: <boost@...>
Sent: Thursday, May 08, 2008 4:03 PM
Subject: Re: [boost] [thread] Expected behaviour of condition
variablewaitwhen the mutex is not locked


> Frank Mori Hess:
>> On Thursday 08 May 2008 04:51 am, Anthony Williams wrote:
>>> To this end, unique_lock is default-constructible and movable, and
>>> supports
>>> lock and unlock operations.
>>
>> Since when is unique_lock default-constructible?  You just mean it has a
>> defer_lock_t constructor, right?
>
> It does have a default constructor. Movable types have to have an "empty"
> state in which to put the moved-from source, and the default constructor
> of
> a movable type usually creates such an "empty" object.

This is right on the proposal, but not onthe Boost::thread library.

> (The proposed promise
> doesn't adhere to this "convention" though.)

Should it?
_____________________
Vicente Juan Botet Escriba


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Re: [thread] Expected behaviour of condition variable waitwhen the mutex is not locked

by Anthony Williams-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

"Peter Dimov" <pdimov@...> writes:

> Frank Mori Hess:
>> On Thursday 08 May 2008 04:51 am, Anthony Williams wrote:
>>> To this end, unique_lock is default-constructible and movable, and
>>> supports
>>> lock and unlock operations.
>>
>> Since when is unique_lock default-constructible?  You just mean it has a
>> defer_lock_t constructor, right?
>
> It does have a default constructor.

The C++0x one does. boost::unique_lock doesn't. I think I'll add one.

> Movable types have to have an "empty"
> state in which to put the moved-from source, and the default constructor of
> a movable type usually creates such an "empty" object.

> (The proposed promise doesn't adhere to this "convention" though.)

I think it should. That would also get rid of the promise_moved
exceptions. I'll post my revised implementation soon.

Anthony
--
Anthony Williams            | Just Software Solutions Ltd
Custom Software Development | http://www.justsoftwaresolutions.co.uk
Registered in England, Company Number 5478976.
Registered Office: 15 Carrallack Mews, St Just, Cornwall, TR19 7UL

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Re: [thread] Expected behaviour of condition variable waitwhen the mutex is not locked

by Frank Mori Hess :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Thursday 08 May 2008 10:03 am, Peter Dimov wrote:
> It does have a default constructor. Movable types have to have an "empty"
> state in which to put the moved-from source, and the default constructor of
> a movable type usually creates such an "empty" object. (The proposed
> promise doesn't adhere to this "convention" though.)

My recollection is that the requirements are looser: a move can leave the
moved-from source in any state it likes as long as it doesn't violate any of
the class' specified invariants.

- --
Frank
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFIIw265vihyNWuA4URAnQbAJ45dbkJv10fONEKvgkyico63DHy9ACfUDVE
rKkCJuCPUDCcgkvnpIYBVfc=
=GLjg
-----END PGP SIGNATURE-----
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Re: [thread] Expected behaviour of condition variablewaitwhen the mutex is not locked

by Vicente Botet Escriba :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


----- Original Message -----
From: "Anthony Williams" <anthony_w.geo@...>
To: <boost@...>
Sent: Thursday, May 08, 2008 3:33 PM
Subject: Re: [boost] [thread] Expected behaviour of condition
variablewaitwhen the mutex is not locked


> "vicente.botet" <vicente.botet@...> writes:
>
>> From: "Anthony Williams" <anthony_w.geo@...>
>
>>> "vicente.botet" <vicente.botet@...> writes:
>>>
>>>> From: "Anthony Williams" <anthony@...>
>>>>> Quoting "vicente.botet" <vicente.botet@...>:
>>>>>
>>>>>> 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.
>
>>> Then you're reading it differently to how it was written. The
>>> precondition
>>> is
>>> "lock is locked by the current thread". If the precondition is violated,
>>> all
>>> bets are off.
>>
>> You are right, as usual. It's the reference implementation described in
>> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2406.html#condition_rationale
>> which induced me on error.
>>
>> What do you think to add the raison you have prefered to abort the
>> program
>> instead of throwing an exception on the rationale of your documentation?
>
> I think it's a logic error in the application, so should be detected as
> soon
> as possible during testing. You can always change it to throw by using the
> "throw on assert" option for Boost.Assert.

Thanks. I have missed this point.

>> The following wait implementation works with models of strict locks. The
>> user is able to add the runtime checks adding the corresponding defines.
>> This implementation do not suffer from undefined behaviour when:
>> * Different mutexes were supplied for concurrent wait() or timedwait()
>> operations on the same condition variable.
>> * The mutex was not owned by the current thread at the time of the call.
>>
>> But
>> * needs to store the reference to the mutex on the condition_variable.
>> When
>> the user do not defines them the behaviour is equivalent.
>> * Strict Lock must provide in addition
>>     - owns_lock
>>     - is_locking, and
>>     - specialize is_strict_lock
>>
>> What is wrong on this approach?
>>
>>
>>     template <class Locker>
>>     void wait(Locker& l) {
>>         BOOST_CONCEPT_ASSERT((StrictLockConcept<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
>>     }
>
> You could do those checks with unique_lock.

Do you mean that it will useful to do this checks with unique_lock?

>>>> 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.
>>>
>>> The wait must unlock the mutex and then lock it again. By using
>>> unique_lock
>>> (which supports that), this possibility is explicit.
>>
>>>From the Thread documentation:
>> "
>> Effects:
>> Atomically call lock.unlock() and blocks the current thread. The thread
>> will
>> unblock when notified by a call to this->notify_one() or
>> this->notify_all(),
>> or spuriously. When the thread is unblocked (for whatever reason), the
>> lock
>> is reacquired by invoking lock.lock() before the call to wait returns.
>> The
>> lock is also reacquired by invoking lock.lock() if the function exits
>> with
>> an exception.
>> "
>>
>> Here it is your implementation
>>
>> inline void condition_variable::wait(unique_lock<mutex>& m)
>> {
>>     detail::interruption_checker check_for_interruption(&cond);
>>     BOOST_VERIFY(!pthread_cond_wait(&cond,m.mutex()->native_handle()));
>> }
>>
>> I don't see any call to m.unlock()/m.lock(). Should you change the
>> Effects
>> section?
>
> No. That's the pthread version, where the OS does the unlock/lock. The
> Windows
> version has to do that explicitly. Since this version only takes
> unique_lock<mutex>, it can rely on knowing what the implementation of
> unique_lock<mutex> looks like for optimization (as in this
> implementation).

OK.

>> Why we can not have:
>>
>> inline void condition_variable::wait(strict_lock<mutex>& m)
>> {
>>     detail::interruption_checker check_for_interruption(&cond);
>>     BOOST_VERIFY(!pthread_cond_wait(&cond,m.mutex()->native_handle()));
>> }
>
> If your strict lock exposes the mutex with a .mutex() member function, you
> can't guarantee strictness.

Yes we can as explained below, if this function is private and the
condition_variable is a friend class.

>> I'm not asking to have a single condition_variable, neither asking to add
>>     void condition_variable_any::wait(strict_lock<mutex>& m)
>>
>> We can have a strict_lock which define this lock/unlock operations
>> private
>> and declares friend condition_variable_any.
>
> Yes, you could.
>
>> Evidently this strict_lock will not be locked while waiting on a
>> condition
>> variable and the operation own_lock will be erroneus. But this operation
>> can
>> not be called because the thread is waiting on the condition. Note that
>> the
>> current implementation of condition_variable::wait(unique_lock<mutex>& m)
>> produce the same symptomes on the unique_lock.
>
> True.
>
>>> The Boost 1.35 "strict lock" is boost::lock_guard.
>> The raison we can not use a lock_guard is that it not provides the mutex
>> operation. It will be enough that lock_guard declares mutex as private
>> operation and make condition_variable a friend class.
>>
>> Next follows some additions to lock_guard that without changing the
>> public
>> interface, allows to use it with a condition_variable_any.
>
>> With this lock_guard, is there any deep raison to don't have:
>>
>> void condition_variable::wait(lock_guard<mutex>& m);
>
> I guess not. I'll think about it.

Thanks

Vicente



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Re: [thread] Expected behaviour of condition variable waitwhen the mutex is not locked

by Anthony Williams-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Anthony Williams <anthony_w.geo@...> writes:

> "Peter Dimov" <pdimov@...> writes:
>
>> Frank Mori Hess:
>>> On Thursday 08 May 2008 04:51 am, Anthony Williams wrote:
>>>> To this end, unique_lock is default-constructible and movable, and
>>>> supports
>>>> lock and unlock operations.
>>>
>>> Since when is unique_lock default-constructible?  You just mean it has a
>>> defer_lock_t constructor, right?
>>
>> It does have a default constructor.
>
> The C++0x one does. boost::unique_lock doesn't. I think I'll add one.

I've added it to trunk.

Anthony
--
Anthony Williams            | Just Software Solutions Ltd
Custom Software Development | http://www.justsoftwaresolutions.co.uk
Registered in England, Company Number 5478976.
Registered Office: 15 Carrallack Mews, St Just, Cornwall, TR19 7UL

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Re: [thread] Expected behaviour of condition variable waitwhen the mutex is not locked

by Anthony Williams-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Frank Mori Hess <frank.hess@...> writes:

> On Thursday 08 May 2008 10:03 am, Peter Dimov wrote:
>> It does have a default constructor. Movable types have to have an "empty"
>> state in which to put the moved-from source, and the default constructor of
>> a movable type usually creates such an "empty" object. (The proposed
>> promise doesn't adhere to this "convention" though.)
>
> My recollection is that the requirements are looser: a move can leave the
> moved-from source in any state it likes as long as it doesn't violate any of
> the class' specified invariants.

That's the requirement, but what Peter describes is the typical implementation
for a move-only type. If unique_lock left the source holding the lock too then
it wouldn't be unique any more, and everything would go wrong.

Moving from a boost::unique_lock has always left the source empty
(l.mutex()==NULL). The newly-added default constructor allows you to start
with it like that.

Anthony
--
Anthony Williams            | Just Software Solutions Ltd
Custom Software Development | http://www.justsoftwaresolutions.co.uk
Registered in England, Company Number 5478976.
Registered Office: 15 Carrallack Mews, St Just, Cornwall, TR19 7UL

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Re: [thread] Expected behaviour of condition variablewaitwhen the mutex is not locked

by Peter Dimov-5 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Frank Mori Hess:
> My recollection is that the requirements are looser: a move can leave the
> moved-from source in any state it likes as long as it doesn't violate any
> of
> the class' specified invariants.

You're right that in principle a moved-from lock may be left with "owns ==
false" without also setting pm to 0, the equivalent of a lock constructed
with defer_lock.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost