fix(bin): bound the startup-network worker's lock waits by its budget - #5528
kunchenguid merged 2 commits into
Conversation
Fixes kunchenguid#5377 The deferred startup network worker bounded its sweeps with a stage budget but took the publish lock and the fleet-lock lease with an unbounded wait, so a live holder of that lock kept the detached worker alive for hours past its timeout with its output discarded at the end. Every wait now goes through the bounded acquire and shares the remaining stage or delivery budget; a lock a live process still holds at the deadline ends the worker with a failed record naming the holder and the rerun command, and a wake so the result surfaces.
|
Speaking as Kun's firstmate: Contract-class: VISION.md (each rule)
aligns when / resist when: Aligns — strengthens a refusal path and survives lock-contention. Does not widen into wake-dedupe (#5378). Closes check: Author Fixes #5377 verified against issue body (25h+ worker past budget on unbounded publish lock) and tip diff. Issue labeled ready-for-pr. CI/NM: all SUCCESS. Attestation MATCH on HEAD |
|
Speaking as Kun's firstmate: this is merged. Thank you @karotkriss — really appreciate you taking the time on this. |
…kunchenguid#5528) * fix(bin): bound the startup-network worker's lock waits by its budget Fixes kunchenguid#5377 The deferred startup network worker bounded its sweeps with a stage budget but took the publish lock and the fleet-lock lease with an unbounded wait, so a live holder of that lock kept the detached worker alive for hours past its timeout with its output discarded at the end. Every wait now goes through the bounded acquire and shares the remaining stage or delivery budget; a lock a live process still holds at the deadline ends the worker with a failed record naming the holder and the rerun command, and a wake so the result surfaces. * no-mistakes(review): propagate publish exit code from cmd_run terminal paths
…kunchenguid#5528) * fix(bin): bound the startup-network worker's lock waits by its budget Fixes kunchenguid#5377 The deferred startup network worker bounded its sweeps with a stage budget but took the publish lock and the fleet-lock lease with an unbounded wait, so a live holder of that lock kept the detached worker alive for hours past its timeout with its output discarded at the end. Every wait now goes through the bounded acquire and shares the remaining stage or delivery budget; a lock a live process still holds at the deadline ends the worker with a failed record naming the holder and the rerun command, and a wake so the result surfaces. * no-mistakes(review): propagate publish exit code from cmd_run terminal paths
…kunchenguid#5528) * fix(bin): bound the startup-network worker's lock waits by its budget Fixes kunchenguid#5377 The deferred startup network worker bounded its sweeps with a stage budget but took the publish lock and the fleet-lock lease with an unbounded wait, so a live holder of that lock kept the detached worker alive for hours past its timeout with its output discarded at the end. Every wait now goes through the bounded acquire and shares the remaining stage or delivery budget; a lock a live process still holds at the deadline ends the worker with a failed record naming the holder and the rerun command, and a wake so the result surfaces. * no-mistakes(review): propagate publish exit code from cmd_run terminal paths
…kunchenguid#5528) * fix(bin): bound the startup-network worker's lock waits by its budget Fixes kunchenguid#5377 The deferred startup network worker bounded its sweeps with a stage budget but took the publish lock and the fleet-lock lease with an unbounded wait, so a live holder of that lock kept the detached worker alive for hours past its timeout with its output discarded at the end. Every wait now goes through the bounded acquire and shares the remaining stage or delivery budget; a lock a live process still holds at the deadline ends the worker with a failed record naming the holder and the rerun command, and a wake so the result surfaces. * no-mistakes(review): propagate publish exit code from cmd_run terminal paths
…kunchenguid#5528) * fix(bin): bound the startup-network worker's lock waits by its budget Fixes kunchenguid#5377 The deferred startup network worker bounded its sweeps with a stage budget but took the publish lock and the fleet-lock lease with an unbounded wait, so a live holder of that lock kept the detached worker alive for hours past its timeout with its output discarded at the end. Every wait now goes through the bounded acquire and shares the remaining stage or delivery budget; a lock a live process still holds at the deadline ends the worker with a failed record naming the holder and the rerun command, and a wake so the result surfaces. * no-mistakes(review): propagate publish exit code from cmd_run terminal paths
…kunchenguid#5528) * fix(bin): bound the startup-network worker's lock waits by its budget Fixes kunchenguid#5377 The deferred startup network worker bounded its sweeps with a stage budget but took the publish lock and the fleet-lock lease with an unbounded wait, so a live holder of that lock kept the detached worker alive for hours past its timeout with its output discarded at the end. Every wait now goes through the bounded acquire and shares the remaining stage or delivery budget; a lock a live process still holds at the deadline ends the worker with a failed record naming the holder and the rerun command, and a wake so the result surfaces. * no-mistakes(review): propagate publish exit code from cmd_run terminal paths
…kunchenguid#5528) * fix(bin): bound the startup-network worker's lock waits by its budget Fixes kunchenguid#5377 The deferred startup network worker bounded its sweeps with a stage budget but took the publish lock and the fleet-lock lease with an unbounded wait, so a live holder of that lock kept the detached worker alive for hours past its timeout with its output discarded at the end. Every wait now goes through the bounded acquire and shares the remaining stage or delivery budget; a lock a live process still holds at the deadline ends the worker with a failed record naming the holder and the rerun command, and a wake so the result surfaces. * no-mistakes(review): propagate publish exit code from cmd_run terminal paths
Intent
Fixes #5377
The deferred startup network worker bounds its sweep work with stage budgets but still calls an unbounded lock wait for publication and delivery, so a live holder of the publish lock can keep the detached worker alive for hours past its timeout, burning CPU with its output discarded.
Make every wait in that worker respect its budget: when the lock cannot be taken in time the worker stops within budget and leaves a durable diagnose-for-rerun result instead of spinning.
This closes upstream issue #5377.
What Changed
take_lockhelper, replacing the unboundedfm_lock_acquire_waitcalls so a live lock holder can no longer keep the detached worker spinning past its timeout with its output discarded.stage_deadline/DELIVERY_DEADLINEbudgets and apublish_lock_heldpath: when a lock cannot be acquired in time, the worker writes a durablefailed/rerunNETWORK_CHECKSrecord naming the holder (FM_LOCK_HELD_PID) and rerun command, queues a wake, and exits non-zero;cmd_runnow propagates that exit code andawait_deliveryloops on the delivery deadline instead of a fixed iteration cap.publishintorecord_result(file writing) plus lock/deadline handling, centralize cleanup inrun_cleanup, updatedocs/configuration.mdto document the bounded lock waits, and add a test that holds the publish lock from a live process and asserts a failed-rerun record instead of a hang.Risk Assessment
✅ Low: Well-bounded shell change that routes every worker lock wait through a budget-bounded acquire, records a durable failed-rerun result on refusal, and is covered by a behavior-based regression test; the minimal fix-round commit correctly propagates publish's exit code and introduces no new defect.
Testing
I ran the full fm-startup-network behavior suite against the fixed worker: all 21 assertions pass, including the new regression that drives the real fm-startup-network.sh run/start worker under a live publish-lock holder held from a separate process. To prove the regression is real I swapped in the base (pre-fix) worker script and re-ran: the new test fails with the worker still waiting on the held publish lock 15s past a 2s budget, then restored the fixed script and confirmed a clean worktree. The test asserts observable behavior end-to-end - the worker gives up in under 6s on a 2s budget, records state=failed, the report output names the holding pid and the exact rerun command, the pre-sweep case runs no sweeps, the post-sweep case preserves the sweep output (PROBE_RAN), and both queue a startup-network wake. The docs/configuration.md line is a one-line comment with no live surface.
Evidence: fm-startup-network bounded lock-wait: fail-before / pass-after
Source: fm-startup-network bounded lock-wait: fail-before / pass-after
## After fix (HEAD=75b9a11): ok - fm-startup-network: a held publish lock ends the worker inside its budget with a failed-rerun record ## Before fix (base script from d4f3b78): not ok - the worker was still waiting on the held publish lock 15s past a 2s budgetPipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
bin/fm-startup-network.sh:596- The new doc line (bin/fm-startup-network.sh:64-65) promisesrun"Exits non-zero when the stage was refused or could not publish, including a lock a live process still held at its deadline." The two explicit refusal paths honor this (registration lock line 501, lease lock line 558 bothreturn 1). But the terminal publish paths (line 596/600/605) ignorepublish's return value and fall through toreturn 0(line 609), so when the publication lock is held past DELIVERY_DEADLINE,publish->publish_lock_heldwrites the failed-rerun record yet cmd_run exits 0. A manualrunfrom the lock-owning harness checking$?sees success despite a failed record. Remedy is trivial (propagate publish's rc, or narrow the doc claim); a durable record is still written either way, so this is cosmetic.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-startup-network.test.sh(fixed worker) - all 21 assertions pass including test_a_held_publish_lock_cannot_keep_the_worker_alive_past_its_budgetSwapped base worker (git show d4f3b78:bin/fm-startup-network.sh) and re-ran the suite: new regression fails with 'still waiting on the held publish lock 15s past a 2s budget'git checkoutrestored fixed worker;git statusconfirms clean worktree at HEAD 75b9a11✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.