From 2109b8c831773ccdeeef65fa44ed9822990d49f8 Mon Sep 17 00:00:00 2001 From: GNERSIS Date: Mon, 14 Sep 2026 18:39:33 +0100 Subject: [PATCH 1/2] fix(pj_base): store the watchdog handle before the watchdog can read it JobControl::armWatchdog started the watchdog thread and then move-assigned it into the job state without holding watchdog_mu. An on_expire callback that re-arms from the watchdog thread reads that handle in the self-re-arm guard. If the body thread was preempted for longer than the timeout between thread creation and the store, the guard read an empty handle, and the re-arm then joined or assigned over a live thread (std::terminate). The handle is now stored under watchdog_mu, which the watchdog thread takes before anything else. ThreadSanitizer reported the race in ProviderJobTest.RearmingTheWatchdogFromOnExpireIsANoOp on every run (20/20). With the fix it reports nothing in 20 runs, and descriptor_import_provider_job_test passes 19/19 under TSan. --- CHANGELOG.md | 17 +++++++++++++++++ pj_base/src/source/provider_job.cpp | 29 ++++++++++++++++------------- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9233c011..b84ae32b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,23 @@ All notable changes to `plotjuggler_sdk` are recorded here. Versioning policy is in [`CLAUDE.md`](./CLAUDE.md) → "Release Versioning". +## [0.34.1] + +Host contract: unchanged (no floor impact). + +### Fix: `armWatchdog` stores the watchdog handle before the watchdog can read it (PATCH) + +`JobControl::armWatchdog` started the watchdog thread and then move-assigned it +into the job state without holding `watchdog_mu`. An `on_expire` callback that +re-arms from the watchdog thread reads that handle for its self-re-arm guard, so +a body thread preempted for longer than the timeout between thread creation and +the store let the guard read an empty handle: the re-arm then joined or +overwrote a thread it should have left alone (`std::terminate` on assigning over +a joinable thread). The store now happens under `watchdog_mu`, which the +watchdog takes before anything else. ThreadSanitizer reported it in +`ProviderJobTest.RearmingTheWatchdogFromOnExpireIsANoOp` on every run. No API or +ABI change. + ## [0.34.0] Host contract: extended: PJ_DATA_PROCESSOR_FLAG_HISTORY_EXEMPT (floor 0.34.0). diff --git a/pj_base/src/source/provider_job.cpp b/pj_base/src/source/provider_job.cpp index 51dfc761..0f22b0ff 100644 --- a/pj_base/src/source/provider_job.cpp +++ b/pj_base/src/source/provider_job.cpp @@ -196,23 +196,26 @@ void JobControl::armWatchdog(std::chrono::milliseconds timeout, std::function lock(state_.watchdog_mu); state_.watchdog_stop = false; + state_.watchdog = std::thread([state, timeout, on_expire = std::move(on_expire), &started]() { + std::unique_lock lock(state->watchdog_mu); + started.release(); + const bool stopped = state->watchdog_cv.wait_for(lock, timeout, [state] { return state->watchdog_stop; }); + lock.unlock(); + if (!stopped) { + try { + on_expire(); + } catch (...) {} + } + }); } - detail::JobState* state = &state_; - std::binary_semaphore started{0}; - state_.watchdog = std::thread([state, timeout, on_expire = std::move(on_expire), &started]() { - std::unique_lock lock(state->watchdog_mu); - started.release(); - const bool stopped = state->watchdog_cv.wait_for(lock, timeout, [state] { return state->watchdog_stop; }); - lock.unlock(); - if (!stopped) { - try { - on_expire(); - } catch (...) {} - } - }); started.acquire(); } From df92ae9b528336b4196a7a165278b0c16c18b8ba Mon Sep 17 00:00:00 2001 From: GNERSIS Date: Mon, 14 Sep 2026 18:52:42 +0100 Subject: [PATCH 2/2] fix(pj_base): rename the watchdog store lock that shadowed the thread's lock The guard held across the handle store was named lock, like the unique_lock the watchdog lambda declares, and GCC's -Wshadow rejected the build under -Werror. It is now store_lock; behaviour is unchanged. --- pj_base/src/source/provider_job.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pj_base/src/source/provider_job.cpp b/pj_base/src/source/provider_job.cpp index 0f22b0ff..e16d9800 100644 --- a/pj_base/src/source/provider_job.cpp +++ b/pj_base/src/source/provider_job.cpp @@ -202,7 +202,7 @@ void JobControl::armWatchdog(std::chrono::milliseconds timeout, std::function lock(state_.watchdog_mu); + std::lock_guard store_lock(state_.watchdog_mu); state_.watchdog_stop = false; state_.watchdog = std::thread([state, timeout, on_expire = std::move(on_expire), &started]() { std::unique_lock lock(state->watchdog_mu);