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..e16d9800 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); + // Held across the handle store: the watchdog takes watchdog_mu before + // anything else, so an on_expire that re-enters armWatchdog reads + // state_.watchdog only after this thread has assigned it. + 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); + 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(); }