fix(supervision): fingerprint lock identity from /proc start_ticks, not ps lstart#621
Open
noahjobse wants to merge 1 commit into
Open
fix(supervision): fingerprint lock identity from /proc start_ticks, not ps lstart#621noahjobse wants to merge 1 commit into
noahjobse wants to merge 1 commit into
Conversation
…ot ps lstart The supervision lock-identity fingerprint (fm_pid_identity) was derived from `ps -p <pid> -o lstart=`. lstart is not a stable primitive: it is derived from the kernel's btime plus the process's starttime ticks. starttime never moves, but btime is recomputed and jitters (observed ~1s, roughly every 30s, on a WSL2 host). Every btime jump shifts the derived lstart of every live process, so a live lock holder's fingerprint drifts against its own process with no second writer. The health predicate (fm_watcher_lock_matches_pid) then misreads a healthy, beating, lock-holding watcher as not-ours and raises false "supervision off" alarms, and a forced re-arm could act on the wrong process. Fingerprint instead from /proc/<pid>/stat field 22 (start ticks), which is boot-relative, carries no btime term, and is constant for the process lifetime. A new fm_pid_start_ticks reads it (parsing fields after the last ')' so a comm with spaces or ')' cannot shift the count). fm_pid_identity uses it, with the old lstart read kept only as a fallback where /proc does not exist (macOS); LC_ALL=C is still pinned so the command half stays locale-invariant. The identity string is only written and equality-compared by every consumer (fm-watch, fm-watch-arm, fm-afk-launch, fm-afk-start, fm-supervise-daemon), so this is drop-in. The one transition effect: a watcher/daemon already running at upgrade whose recorded identity is an lstart string re-reads as not-matching once, is treated as stale, and re-records its ticks identity - a single, self-healing event. Tests: two focused cases added to tests/fm-watcher-lock.test.sh - one asserting byte-stability across repeated reads of one live pid, and one asserting the fingerprint is /proc field 22 and stays fixed across a simulated btime shift that the equivalent lstart derivation would move under, so the cure cannot be quietly regressed back to lstart. The existing locale-invariance test still passes. Claude-Session: https://claude.ai/code/session_01PKVpT6RbwBg74REPzHMG4n
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The supervision lock-identity fingerprint (
fm_pid_identityinbin/fm-wake-lib.sh) is derived fromps -p <pid> -o lstart=.lstartis not a stable primitive: the kernel derives it frombtime(boot time) plus the process'sstarttimeticks.starttimenever moves, butbtimeis recomputed and jitters (observed ~1s, roughly every 30s, on a WSL2 host).Every
btimejump shifts the derivedlstartof every live process. So a live lock holder's fingerprint drifts against its own process with no second writer. The health predicatefm_watcher_lock_matches_pidthen misreads a healthy, beating, lock-holding watcher as "not ours" and raises false "supervision off" alarms. A forced re-arm off that misread could then act on the wrong process.This is a self-inflicted flap: nothing external changes, yet supervision reports itself down.
Fix
Fingerprint from
/proc/<pid>/statfield 22 (start ticks) instead. That value is boot-relative, carries nobtimeterm, and is constant for the process lifetime, so it cannot drift.fm_pid_start_ticksreads field 22, counting fields after the last)so acommcontaining spaces or)can't shift the count.fm_pid_identitynow emits<start_ticks> <command>. The oldlstartread is kept only as a fallback where/procdoes not exist (macOS).LC_ALL=Cis still pinned so the command half stays locale-invariant.Compatibility
The identity string is only written and equality-compared by every consumer (
fm-watch,fm-watch-arm,fm-afk-launch,fm-afk-start,fm-supervise-daemon) — none parse its fields — so this is drop-in. The one transition effect: a watcher/daemon already running at upgrade whose recorded identity is anlstartstring re-reads as not-matching once, is treated as stale, and re-records its ticks identity. A single, self-healing event.Verification
Two focused cases added to
tests/fm-watcher-lock.test.sh:/proc/<pid>/statfield 22, and that it stays fixed across a simulatedbtimeshift under which the equivalentlstartderivation (btime + starttime/HZ) provably would move. This pins the cure so a future refactor can't quietly regress it back tolstart. Skips cleanly where/procis absent.The existing
test_pid_identity_is_locale_invariantstill passes. Fulltests/fm-watcher-lock.test.shsuite is green (24 cases, exit 0) on Linux.Verified on the original WSL2 host where the drift was observed: the new fingerprint held byte-identical across a real
btimejitter that shiftedlstart.Scope
Deliberately narrow: the root-cause primitive plus its regression tests. This is extracted from a larger torn-watcher-lock hardening set on the contributing fork; this PR is the focused, reviewable core of it.