From ae6ab66af4abf44b1a77268e5556d083da729798 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 29 Jan 2019 15:09:54 -0800 Subject: [PATCH] fix #755: a race condition cousing a deadlock 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. --- libs/utils/include/utils/JobSystem.h | 3 +-- libs/utils/src/JobSystem.cpp | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/libs/utils/include/utils/JobSystem.h b/libs/utils/include/utils/JobSystem.h index d88d6519e299..0dbe03930069 100644 --- a/libs/utils/include/utils/JobSystem.h +++ b/libs/utils/include/utils/JobSystem.h @@ -70,8 +70,7 @@ class JobSystem { uint16_t parent; // 2 | 2 std::atomic runningJobCount = { 1 }; // 2 | 2 mutable std::atomic refCount = { 1 }; // 2 | 2 - std::atomic_bool hasWaiter = { false }; // 1 | 1 - // 5 | 1 (padding) + // 6 | 2 (padding) // 64 | 64 }; diff --git a/libs/utils/src/JobSystem.cpp b/libs/utils/src/JobSystem.cpp index 1e15496418e3..2c5d7f9d1a6e 100644 --- a/libs/utils/src/JobSystem.cpp +++ b/libs/utils/src/JobSystem.cpp @@ -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; @@ -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 lock(mWaiterLock); - job->hasWaiter.store(true); while (!hasJobCompleted(job) && !exitRequested()) { mWaiterCondition.wait(lock); }