Skip to content

fix: make TERM→KILL escalation poll for group death and document the zombie-probe invariant - #3887

Merged
max-sixty merged 5 commits into
mainfrom
escalation-probe-design
Aug 23, 2026
Merged

fix: make TERM→KILL escalation poll for group death and document the zombie-probe invariant#3887
max-sixty merged 5 commits into
mainfrom
escalation-probe-design

Conversation

@max-sixty

@max-sixty max-sixty commented Aug 23, 2026

Copy link
Copy Markdown
Owner

The TERM→KILL escalation probed group liveness with one killpg(pgid, 0) after a fixed 200 ms sleep, and that probe counts an exited-but-unreaped member as alive (Linux answers Ok, macOS EPERM). On the timed-out-command path the caller deliberately holds the group leader unreaped, so the group read alive for the whole grace and the final group SIGKILL fired every time.

That SIGKILL is inert — a signal to a fully-exited group is discarded and cannot change its recorded exit status (verified empirically, and pinned by a new test) — so this was never a correctness bug: whenever the probe over-reports, the KILL hits nothing; whenever the KILL matters, the group genuinely is alive. The costs were latency and legibility: every escalation step slept its full 200 ms even when the tree was already dead (every Ctrl-C of an isolated streamed child, every wt step tether teardown), and the probe read as a broken correctness gate.

The change keeps the semantics and reframes the roles, matching coreutils timeout -k (which KILLs unconditionally after its grace, with no probe at all):

  • The post-grace SIGKILL is the documented contract: whatever remains after the grace is swept. It is also the only signal that removes a SIGSTOP'd member, which runs no TERM handler.
  • The probe becomes an early-exit optimization: a 20 ms poll with an immediate first probe replaces the fixed sleep. Callers whose children are reaped concurrently (the signal forwarder, tether's supervisor) return within one poll interval; the timed-out path still runs its full grace and sends the inert sweep, which its docstring now states.
  • Signals sent to live groups are identical to before at all three call sites; only signals with provably no live target are skipped, and returns are never later than before.

The correctness argument lives in the docstrings on process_group_alive, forward_signal_with_escalation, and kill_timed_out_tree. Three new unit tests pin the invariants: immediate return for an empty group, full grace on a live group, and — replaying the timed-out sequence with the leader held unreaped — a preserved exit code through both the TERM and the post-grace sweep (barriered on the child's stdout EOF, so no scheduling assumptions).

Testing: the unit tests above plus the existing signal-forwarding and tether integration tests; full pre-merge gate green locally. The full-grace test pins the zombie-probe reading on both Linux and macOS CI (the test (linux)/test (macos) jobs run the lib unit tests).

This was written by Claude Code on behalf of max-sixty

max-sixty and others added 4 commits August 23, 2026 11:44
…zombie-probe invariant

process_group_alive counts an exited-but-unreaped member as alive (Linux: Ok,
macOS: EPERM), so on the timed-out-command path — whose caller holds the group
leader unreaped — the group read alive for the whole grace and the final
SIGKILL always fired. That sweep is provably inert against a dead group
(signals to fully-exited processes are discarded), so the fix reframes rather
than re-plumbs: the probe is now an early-exit optimization (20ms poll,
immediate first probe) and the post-grace SIGKILL is the documented straggler
sweep. Callers whose children are reaped concurrently (Cmd::stream's signal
forwarder, tether) now return within one poll interval instead of always
sleeping the full 200ms per escalation step.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Barrier on the child's stdout EOF (the pipe closes at process exit) instead of
racing TERM delivery against the grace: the child exits on its own before
escalation starts, so no CI-scheduling assumption remains, and the preserved
exit code pins that both the TERM and the post-grace sweep are inert against
an exited-but-unreaped group.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The equivalence argument holds: the next signal in the chain still only fires after the full grace, so signals to live groups land at exactly the same times as before, and the early return is reachable only via ESRCH — which requires every member reaped, and reaping is monotonic. The one place the two differ is pid recycling, where polling is strictly safer than the single end-of-grace probe: a group reaped at 100 ms whose pgid was recycled by 200 ms used to read alive and draw a killpg at a stranger.

One thing worth folding back into the PR body: test (macos) is green on this head, and wt hook pre-merge --yes runs the lib unit tests, so test_escalation_full_grace_and_inert_sweep_when_leader_unreaped executed there. That makes the macOS half of the process_group_alive docstring's zombie claim empirically pinned by CI, not just Linux — the description currently scopes the pinning to Linux.

One non-blocking test suggestion inline.

Comment thread src/shell_exec.rs
A 30s grace makes returning early structurally distinguishable from sleeping
it, so the wall-clock bound no longer races CI scheduling.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@max-sixty
max-sixty merged commit 6aad9e1 into main Aug 23, 2026
38 checks passed
@max-sixty
max-sixty deleted the escalation-probe-design branch August 23, 2026 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants