Skip to content

Commit

Permalink
Bugfix in Core: deadlock in OrderedLock class.
Browse files Browse the repository at this point in the history
If a second thread called `lock` and was waiting on `cvar.back()`, the referenced `std::condition_variable` could be destructed by the first locking thread calling `unlock`, causing the second thread to `wait` forever. This is fixed by `pop`ing the queue only after `wait` completed (i.e. within `lock`).
Additionally, spurious wakeups could cause the `wait` to end prematurely, which is fixed by using the predicated overload.
Also made some naming improvements and fixed formatting.
  • Loading branch information
GPMueller committed Jan 18, 2021
1 parent c97f00b commit fba2fb5
Showing 1 changed file with 14 additions and 17 deletions.
31 changes: 14 additions & 17 deletions core/include/utility/Ordered_Lock.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,35 +8,32 @@

class Ordered_Lock
{
std::queue<std::condition_variable> cvar;
std::mutex cvar_lock;
bool locked;
std::queue<std::condition_variable> condition_;
std::mutex condition_mutex_;
bool locked_;

public:
Ordered_Lock() : locked(false) {};
Ordered_Lock() : locked_( false ){};

void lock()
{
std::unique_lock<std::mutex> acquire(cvar_lock);
if( locked )
std::unique_lock<std::mutex> condition_lock( condition_mutex_ );
if( locked_ )
{
cvar.emplace();
cvar.back().wait(acquire);
condition_.emplace();
condition_.back().wait( condition_lock, [&] { return !locked_; } );
condition_.pop();
}
else
locked = true;
locked_ = true;
}

void unlock()
{
std::unique_lock<std::mutex> acquire(cvar_lock);
if( cvar.empty() )
locked = false;
else
{
cvar.front().notify_one();
cvar.pop();
}
std::unique_lock<std::mutex> condition_lock( condition_mutex_ );
locked_ = false;
if( !condition_.empty() )
condition_.front().notify_one();
}
};

Expand Down

0 comments on commit fba2fb5

Please sign in to comment.