From bd2179b08f0ffd8a9774dabbebc1760ea808d929 Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Mon, 24 Aug 2026 14:32:31 +0200 Subject: [PATCH] Let the parallel pass signal completion under the lock its waiter tests RunTasks keeps its RunState - counter, mutex, condition variable - on its own stack frame and hands a pointer to the pool. The last worker to finish decremented the counter OUTSIDE the mutex and only then took it to notify, while the waiter's predicate was the counter itself. So the waiter could see zero the instant the decrement landed, find its predicate already true, never block, and return from RunTasks - popping the frame. The worker then locked a mutex and signalled a condition variable that no longer existed, writing pthread state into a frame the submitting thread had already reused. pthread_mutex_unlock writes owner and nusers as eight contiguous zero bytes. Land those on a live pointer and the next read of a member at offset 8 faults: the observed crash was fmt's buffer::append with this == nullptr, in the log call immediately after a parallel pass, which is why it always appeared after the resolution-filter line - that line was simply the next thing to use the frame. Give the waiter a flag set under the same lock as the notify. Completion cannot then be observed until the notifier has released the mutex, i.e. after its last touch of the state. The counter keeps its lock-free fast path and decides only who notifies, so there is still exactly one lock per pass. Found independently by two investigations: a widened-window reproducer (2 crashes in 38 unfixed, 0 in 60 fixed; glibc's own "__owner == 0" assertion caught in the pool worker) and an isolated one that clobbered a freshly filled stack frame 732 times in 60000 and never after the fix. Growing RunState by eight bytes, changing nothing else, took the rate from 0/90 to 3 hard failures in 30. The dataset that failed about one run in twelve: 0 of 40. Space group and merged output unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- common/ParallelFor.h | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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); }