fix: gracefully terminate none-runtime process group on cancel#132
Open
zpzjzj wants to merge 1 commit into
Open
fix: gracefully terminate none-runtime process group on cancel#132zpzjzj wants to merge 1 commit into
zpzjzj wants to merge 1 commit into
Conversation
Timed-out or canceled `none` runtime commands previously had their whole process group hit with an immediate SIGKILL. Children got no chance to run trap handlers, release locks, or clean up — so a helper holding a lock could leave stale state that stalled a later command until a long timeout. Cancellation now signals the group with SIGTERM first, then escalates to SIGKILL after a short grace (noneExecKillGrace = 1s) if the group has not exited. The grace fits inside the existing 2s WaitDelay, so a timed-out Exec still returns within its deadline and preserves the context.DeadlineExceeded / exit-code -1 contract. Whole-group semantics (negative PID) and the single-process fallback are kept; escalation runs out of band so Cancel returns immediately, and a done channel (closed once Wait returns) stops the timer before the PID can be recycled. No-op on non-Unix platforms. Adds a SIGTERM-trap test proving the graceful path runs cleanup before teardown, and a lock-file regression test proving a stale child is gone after timeout so a second command does not hang on it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Summary
The
noneruntime already starts commands in their own process group and kills the whole group on cancellation, but it did so with an immediate SIGKILL. Children never got a chance to run trap handlers, release locks, or clean up — the exact failure mode the issue describes, where a helper holding a lock left stale state that stalled a later command.This adds a graceful escalation path:
noneExecKillGrace = 1s), it escalates to SIGKILL.WaitDelay, so a timed-outExecstill returns within its deadline and preserves thecontext.DeadlineExceeded/ exit-code-1contract.cmd.Cancelreturns immediately andos/execcan start itsWaitDelaytimer; adonechannel closed onceWaitreturns stops the escalation timer before the PID can be recycled.none_exec_other.go, guarded by the same build tags).Tests
TestNoneRuntime_ExecTerminatesGracefullyThenEscalates— a SIGTERM trap writes a marker before exit, proving the graceful path runs cleanup rather than an immediate SIGKILL (verified: fails under immediate-SIGKILL, passes with SIGTERM-first).TestNoneRuntime_ExecStaleLockDoesNotHangNextCommand— the issue's lock-file regression: a first command times out while a background child records its PID as a lock; after the timeout the child's group is gone, and a second command sees a dead PID (kill -0) and does not hang.TestNoneRuntime_ExecKillsDescendantsOnTimeoutand the deadline/cancel contract tests still pass.go build ./...,go vet ./...,gofmt -lclean, andgo test -race ./...green.Fixes #120
🤖 Generated with Claude Code