AtomicCondition: fix race condition, remove notify_{one,all}

* Fix race condition in wait_for between calling atomic load
  and condition_variable::wait_for, by protecting atomic store
  with same mutex as AtomicCondition::wait_for.
* Subsequently, simplify the interface by removing notify_{one,all}.
  condition_varaiable::notify_all is now automatically called in
  `AtomicCondition::set(val)`.
This commit is contained in:
Tom 2019-09-23 12:04:28 +02:00
parent 2ecc643bc5
commit 2a04dd5cc1
3 changed files with 14 additions and 18 deletions

View File

@ -18,7 +18,7 @@ class AtomicCondition
public: public:
explicit AtomicCondition(ValueType val) explicit AtomicCondition(ValueType val)
: value_(val) : value_(val)
, condvar_mutex_() , mutex_()
, condvar_() , condvar_()
{ {
} }
@ -30,14 +30,23 @@ public:
void set(ValueType val) noexcept void set(ValueType val) noexcept
{ {
{
// This ensures that wait_for is either
// 1. not running, or
// 2. in a waiting state
// to avoid a data race between value_.load and cond_var.wait_for.
std::unique_lock<std::mutex> lock(this->mutex_);
this->value_.store(val); this->value_.store(val);
} }
this->condvar_.notify_all();
}
template<class Rep, class Period> template<class Rep, class Period>
void wait_for(const std::chrono::duration<Rep, Period>& time, void wait_for(const std::chrono::duration<Rep, Period>& time,
ValueType val) const ValueType val) const
{ {
std::unique_lock<std::mutex> lock(this->condvar_mutex_); std::unique_lock<std::mutex> lock(this->mutex_);
while( this->value_.load() != val ) while( this->value_.load() != val )
if( this->condvar_.wait_for(lock, time) == std::cv_status::timeout ) if( this->condvar_.wait_for(lock, time) == std::cv_status::timeout )
@ -48,26 +57,16 @@ public:
void wait_for(const std::chrono::duration<Rep, Period>& time, void wait_for(const std::chrono::duration<Rep, Period>& time,
Predicate pred) const Predicate pred) const
{ {
std::unique_lock<std::mutex> lock(this->condvar_mutex_); std::unique_lock<std::mutex> lock(this->mutex_);
while( !pred() ) while( !pred() )
if( this->condvar_.wait_for(lock, time) == std::cv_status::timeout ) if( this->condvar_.wait_for(lock, time) == std::cv_status::timeout )
return; return;
} }
void notify_one() noexcept
{
this->condvar_.notify_one();
}
void notify_all() noexcept
{
this->condvar_.notify_all();
}
private: private:
std::atomic<ValueType> value_; std::atomic<ValueType> value_;
mutable std::mutex condvar_mutex_; mutable std::mutex mutex_;
mutable std::condition_variable condvar_; mutable std::condition_variable condvar_;
}; };

View File

@ -58,7 +58,6 @@ public:
it != this->signal_map_.end() ) it != this->signal_map_.end() )
{ {
this->condition_.set(it->second); this->condition_.set(it->second);
this->condition_.notify_all();
return it->first; return it->first;
} }
} }

View File

@ -58,7 +58,6 @@ TEST_CASE("wait-for-value")
std::this_thread::yield(); std::this_thread::yield();
condition.set(42); condition.set(42);
condition.notify_one();
future.get(); future.get();
REQUIRE( condition.get() == 42 ); REQUIRE( condition.get() == 42 );
@ -79,7 +78,6 @@ TEST_CASE("wait-for-predicate")
std::this_thread::yield(); std::this_thread::yield();
condition.set(42); condition.set(42);
condition.notify_one();
future.get(); future.get();
REQUIRE( condition.get() == 42 ); REQUIRE( condition.get() == 42 );