Skip to content

Mast v0.7.8: stop a command even when something it started outlives the TERM - #58

Merged
stackrats merged 2 commits into
mainfrom
fix/stop-waits-for-survivors
Oct 14, 2026
Merged

stackrats merged 2 commits into
mainfrom
fix/stop-waits-for-survivors

Conversation

@stackrats

Copy link
Copy Markdown
Owner

Replaces the v0.7.7 release, which never published: its CI gate failed on the new macOS runner image. The v0.7.7 tag exists but has no release.

What

  • Stopping a command could hang for as long as one of its descendants ran.
    • Mast sends SIGTERM to the command's process group and waits for the direct child. It escalated to SIGKILL only if that child was still alive after the 8 s grace.
    • A descendant that outlived the TERM was never killed. That covers one that ignores TERM, and one caught mid-fork when the signal landed.
    • That descendant still held the command's stdout and stderr. An operation ends only when its output forwarder does, which waited for every holder of the pipes to close them.
  • Why v0.7.7's gate failed: macOS runner image macos-26-arm64 20261013 makes the mid-fork case routine. project_removal_blocks_new_commands_until_shutdown_finishes failed on all three runs that landed on it and passed on the September image.
  • Fix 1: once the direct child has exited, the rest of its group gets what's left of the grace, then SIGKILL.
  • Fix 2: after a command exits, its output is forwarded for at most 2 more seconds. A background process a command deliberately leaves running also inherits the pipes, and the operation stayed open until that process ended.

Verified

  • cancellation_stops_a_descendant_that_outlives_the_term (new, mast-docker): a background sleeper ignores SIGTERM and holds the pipes.
    • Without fix 1: "the descendant that ignored SIGTERM outlived the cancel".
    • With it: cancel returns in about 1 s and the sleeper is gone.
  • a_command_that_leaves_a_background_process_still_finishes (new, engine): a script backgrounds sleep 300 and exits.
    • Without fix 2: still open after 15 s.
    • With it: Completed in about 2 s.
  • The traced removal scenario, repeated 15 times on the new macOS image, via draft [debug] Trace the removal test on the new macOS runner image — do not merge #57 with this fix applied. See the result there before merging.
  • cargo test --workspace passes.
  • cargo clippy --workspace --all-targets -D warnings is clean on Rust 1.96 and 1.99.

Stopping a command sends SIGTERM to its process group and waits for the
direct child, escalating to SIGKILL only if that child is still there
after the grace. A descendant that outlived the TERM — one that ignores
it, or one caught mid-fork when it landed — was never killed, and it
still held the command's stdout and stderr. The operation ends only
when its output forwarder does, which waited for every holder of those
pipes to close them, so the stop waited for as long as the survivor
ran.

macOS runner image 20261013 makes the mid-fork case routine: in
project_removal_blocks_new_commands_until_shutdown_finishes the test
script's background `sleep 60` survived the TERM, the shell exited on
release, and the command's operation stayed open past the test's thirty
seconds — failing v0.7.7's release gate on all three runs there. A
traced run showed the shell gone, the sleep alive in the command's
process group with init as its parent, and no event from the command at
all. The cancellation test in mast-docker leaked a sleeper the same
way; it only checked that cancel returned promptly.

Two fixes, each with a test that fails without it:

- Once the direct child has exited, the rest of its group gets what is
  left of the grace to follow it, then SIGKILL.
- When a command exits, its output is forwarded for at most two more
  seconds. A background process a command leaves running on purpose
  also inherits the pipes, and the operation used to stay open until
  that process ended.
@stackrats
stackrats merged commit ee2a960 into main Oct 14, 2026
6 checks passed
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.

1 participant