Skip to content

Commit

Permalink
fix #755: a race condition cousing a deadlock
Browse files Browse the repository at this point in the history
This reverts a JobSystem optimization that attempted to avoid signaling
a condition when there was no waiters. Unfortunately, there was a
race that caused the the signaling thread to miss that the waiter flag
was set, thus not signaling.
  • Loading branch information
pixelflinger committed Jan 29, 2019
1 parent 7be3994 commit ae6ab66
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 4 deletions.
3 changes: 1 addition & 2 deletions libs/utils/include/utils/JobSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ class JobSystem {
uint16_t parent; // 2 | 2
std::atomic<uint16_t> runningJobCount = { 1 }; // 2 | 2
mutable std::atomic<uint16_t> refCount = { 1 }; // 2 | 2
std::atomic_bool hasWaiter = { false }; // 1 | 1
// 5 | 1 (padding)
// 6 | 2 (padding)
// 64 | 64
};

Expand Down
3 changes: 1 addition & 2 deletions libs/utils/src/JobSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ void JobSystem::finish(Job* job) noexcept {
std::atomic_thread_fence(std::memory_order_acquire);
#endif
// no more work, destroy this job and notify its the parent
notify = notify || job->hasWaiter.load();
notify = true;
Job* const parent = job->parent == 0x7FFF ? nullptr : &storage[job->parent];
decRef(job);
job = parent;
Expand Down Expand Up @@ -427,7 +427,6 @@ void JobSystem::waitAndRelease(Job*& job) noexcept {
// test if job has completed first, to possibly avoid taking the lock
if (!hasJobCompleted(job)) {
std::unique_lock<Mutex> lock(mWaiterLock);
job->hasWaiter.store(true);
while (!hasJobCompleted(job) && !exitRequested()) {
mWaiterCondition.wait(lock);
}
Expand Down

0 comments on commit ae6ab66

Please sign in to comment.