diff --git a/common/ParallelFor.h b/common/ParallelFor.h index 682ab999..498e01cf 100644 --- a/common/ParallelFor.h +++ b/common/ParallelFor.h @@ -101,6 +101,7 @@ namespace parallel_detail { std::atomic remaining{0}; std::mutex done_m; std::condition_variable done_cv; + bool done = false; // guarded by done_m; see RunOneTask std::mutex err_m; std::exception_ptr error; }; @@ -113,7 +114,15 @@ namespace parallel_detail { if (!s.error) s.error = std::current_exception(); } if (s.remaining.fetch_sub(1, std::memory_order_acq_rel) == 1) { + // The flag the waiter tests is set UNDER done_m, and the counter is not that flag. If the + // waiter watched the counter it could see zero the instant the decrement above lands - + // before this thread has taken the lock - find its predicate already true, never block, + // and return from RunTasks. RunState is a local of that frame, so the lock and the notify + // below would then run on a destroyed mutex and condition variable, writing pthread state + // into a stack frame the submitting thread has already reused. Watching a flag set under + // the lock means completion cannot be observed until this thread has released it. std::lock_guard lock(s.done_m); + s.done = true; s.done_cv.notify_all(); } } @@ -137,7 +146,7 @@ namespace parallel_detail { RunOneTask(s, 0); { std::unique_lock lock(s.done_m); - s.done_cv.wait(lock, [sp] { return sp->remaining.load(std::memory_order_acquire) == 0; }); + s.done_cv.wait(lock, [sp] { return sp->done; }); } if (s.error) std::rethrow_exception(s.error); }