857. condition_variable::time_wait return bool error prone

Section: 32.7.4 [thread.condition.condvar] Status: C++11 Submitter: Beman Dawes Opened: 2008-06-13 Last modified: 2016-01-28

Priority: Not Prioritized

View all other issues in [thread.condition.condvar].

View all issues with C++11 status.

Discussion:

The meaning of the bool returned by condition_variable::timed_wait is so obscure that even the class' designer can't deduce it correctly. Several people have independently stumbled on this issue.

It might be simpler to change the return type to a scoped enum:

enum class timeout { not_reached, reached };

That's the same cost as returning a bool, but not subject to mistakes. Your example below would be:

if (cv.wait_until(lk, time_limit) == timeout::reached )
  throw time_out();

[ Beman to supply exact wording. ]

[ San Francisco: ]

There is concern that the enumeration names are just as confusing, if not more so, as the bool. You might have awoken because of a signal or a spurious wakeup, for example.

Group feels that this is a defect that needs fixing.

Group prefers returning an enum over a void return.

Howard to provide wording.

[ 2009-06-14 Beman provided wording. ]

[ 2009-07 Frankfurt: ]

Move to Ready.

Proposed resolution:

Change Condition variables 32.7 [thread.condition], Header condition_variable synopsis, as indicated:

namespace std {
  class condition_variable;
  class condition_variable_any;

  enum class cv_status { no_timeout, timeout };
}

Change class condition_variable 32.7.4 [thread.condition.condvar] as indicated:

class condition_variable { 
public:
  ...
  template <class Clock, class Duration>
    bool cv_status wait_until(unique_lock<mutex>& lock,
                    const chrono::time_point<Clock, Duration>& abs_time);
  template <class Clock, class Duration, class Predicate>
    bool wait_until(unique_lock<mutex>& lock,
                    const chrono::time_point<Clock, Duration>& abs_time,
                    Predicate pred);

  template <class Rep, class Period>
    bool cv_status wait_for(unique_lock<mutex>& lock,
                  const chrono::duration<Rep, Period>& rel_time);
  template <class Rep, class Period, class Predicate>
    bool wait_for(unique_lock<mutex>& lock,
                  const chrono::duration<Rep, Period>& rel_time,
                  Predicate pred);
  ...
};

...

template <class Clock, class Duration>
  bool cv_status wait_until(unique_lock<mutex>& lock,
                  const chrono::time_point<Clock, Duration>& abs_time);

-15- Precondition: lock is locked by the calling thread, and either

• no other thread is waiting on this condition_variable object or
• lock.mutex() returns the same value for each of the lock arguments supplied by all concurrently waiting threads (via wait, wait_for or wait_until.).

-16- 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 signaled by a call to notify_one(), a call to notify_all(), by the current time exceeding abs_time if Clock::now() >= abs_time, or spuriously.
• If the function exits via an exception, lock.unlock() shall be called prior to exiting the function scope.

-17- Postcondition: lock is locked by the calling thread.

-18- Returns: Clock::now() < abs_time cv_status::timeout if the function unblocked because abs_time was reached, otherwise cv_status::no_timeout.

-19- Throws: std::system_error when the effects or postcondition cannot be achieved.

-20- Error conditions:

• operation_not_permitted — if the thread does not own the lock.
• equivalent error condition from lock.lock() or lock.unlock().

template <class Rep, class Period>
  bool cv_status wait_for(unique_lock<mutex>& lock,
                const chrono::duration<Rep, Period>& rel_time);

-21- Effects Returns:

wait_until(lock, chrono::monotonic_clock::now() + rel_time)

-22- Returns: false if the call is returning because the time duration specified by rel_time has elapsed, otherwise true.

[ This part of the wording may conflict with 859 in detail, but does not do so in spirit. If both issues are accepted, there is a logical merge. ]

template <class Clock, class Duration, class Predicate> 
  bool wait_until(unique_lock<mutex>& lock, 
                  const chrono::time_point<Clock, Duration>& abs_time, 
                  Predicate pred);

-23- Effects:

while (!pred()) 
  if (!wait_until(lock, abs_time) == cv_status::timeout) 
    return pred(); 
return true;

-24- Returns: pred().

-25- [Note: The returned value indicates whether the predicate evaluates to true regardless of whether the timeout was triggered. — end note].

Change Class condition_variable_any 32.7.5 [thread.condition.condvarany] as indicated:

class condition_variable_any {
public:
  ...
  template <class Lock, class Clock, class Duration>
    bool cv_status wait_until(Lock& lock,
                    const chrono::time_point<Clock, Duration>& abs_time);
  template <class Lock, class Clock, class Duration, class Predicate>
    bool wait_until(Lock& lock,
                    const chrono::time_point<Clock, Duration>& abs_time,
                    Predicate pred);

  template <class Lock, class Rep, class Period>
    bool cv_status wait_for(Lock& lock,
                  const chrono::duration<Rep, Period>& rel_time);
  template <class Lock, class Rep, class Period, class Predicate>
    bool wait_for(Lock& lock,
                  const chrono::duration<Rep, Period>& rel_time,
                  Predicate pred);
  ...
};

...

template <class Lock, class Clock, class Duration>
  bool cv_status wait_until(Lock& lock,
                  const chrono::time_point<Clock, Duration>& abs_time);

-13- 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 signaled by a call to notify_one(), a call to notify_all(), by the current time exceeding abs_time if Clock::now() >= abs_time, or spuriously.
• If the function exits via an exception, lock.unlock() shall be called prior to exiting the function scope.

-14- Postcondition: lock is locked by the calling thread.

-15- Returns: Clock::now() < abs_time cv_status::timeout if the function unblocked because abs_time was reached, otherwise cv_status::no_timeout.

-16- Throws: std::system_error when the effects or postcondition cannot be achieved.

-17- Error conditions:

• equivalent error condition from lock.lock() or lock.unlock().

template <class Lock, class Rep, class Period>
  bool cv_status wait_for(Lock& lock,
                const chrono::duration<Rep, Period>& rel_time);

-18- Effects Returns:

wait_until(lock, chrono::monotonic_clock::now() + rel_time)

-19- Returns: false if the call is returning because the time duration specified by rel_time has elapsed, otherwise true.

[ This part of the wording may conflict with 859 in detail, but does not do so in spirit. If both issues are accepted, there is a logical merge. ]

template <class Lock, class Clock, class Duration, class Predicate> 
  bool wait_until(Lock& lock, 
                  const chrono::time_point<Clock, Duration>& rel_time abs_time, 
                  Predicate pred);

-20- Effects:

while (!pred()) 
  if (!wait_until(lock, abs_time) == cv_status::timeout) 
    return pred(); 
return true;

-21- Returns: pred().

-22- [Note: The returned value indicates whether the predicate evaluates to true regardless of whether the timeout was triggered. — end note].