fix(runner): release both teardowns when Ctrl-C stops a parallel run - #1336
Merged
Chemaclass merged 2 commits intoAug 21, 2026
Merged
Conversation
Under `--parallel`, Ctrl-C ran neither the per-test `tear_down` nor the file's `tear_down_after_script`. `main::cleanup` pkills the runner's children, which kills the file worker outright: since #1320 that worker owns the file hook, and the parent cannot run it in its place because several files are in flight and the hook is unset and redefined as the loop advances. The worker now traps the signal itself and settles the debt the parent recorded before dispatch. The debt is cleared before the hook runs, so a signal landing while the worker is already inside its teardown cannot run it twice. Measured rather than assumed, and three things differ from the issue's model: - The test body subshell is a great-grandchild of the runner, not a grandchild (runner, file worker, run_test fork, body subshell, leaf command). - SIGINT cannot be covered here. A shell sets SIGINT to SIG_IGN in a job it backgrounds, and a signal ignored on entry can be neither trapped nor reset, on bash 3.2 and 5.3 alike. Ctrl-C arrives as the SIGTERM the parent pkills with, so TERM is the only disposition worth a handler. - A per-pid TERM does not reach a test body: bash defers the trap until the running foreground command returns, so a body inside the test's own `sleep` never reaches the EXIT trap where `tear_down` lives. Each test therefore gets its own process group, and the handler signals the group, which is the idiom run_with_timeout already uses for the same reason. The handler deliberately does not sweep with `pkill -P $$`. `$$` stays the runner's pid inside a subshell, so the sweep signalled the runner's children, this worker among them, and killed the handler before it reached the hook. Bash 3.2 won that race and bash 5 lost it every time. `BASHPID` would name the worker and is Bash 4+. The cost is that a file opting out of per-test parallelism runs its bodies unforked, with no group of their own, so their `tear_down` is missed. Best effort, as in #1323: a hook that never returns does not hold the run, which still prints its message and exits 1 promptly. Closes #1331
Measured rather than assumed: such a file runs its bodies unforked, so the worker sits in a command substitution and bash defers the trap until it returns. Both hooks still run, when the body finishes rather than when the signal lands, so nothing is leaked and the interrupt only fails to cut that test short.
Chemaclass
deleted the
fix/1331-ctrl-c-runs-neither-teardown-under-parallel
branch
August 21, 2026 13:01
5 tasks
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.
🤔 Background
Related #1331
Under
--parallel, Ctrl-C released neither the per-testtear_downnor the file'stear_down_after_script. The parent's kill takes the file worker down outright, and since #1320 that worker is the only frame holding the file's hook.💡 Changes
sleepnever reaches the EXIT trap wheretear_downlives.SIG_IGNin a job it backgrounds, and a signal ignored on entry can be neither trapped nor reset. Measured the same on bash 3.2 and 5.3. Cleanup reaches this frame as the SIGTERM the parent sends, so the issue's note asking for INT as well cannot be honoured and does not need to be.pkill -P $$sweep is possible in the worker, because$$names the runner inside a subshell and the sweep killed the handler itself.tear_down_after_scriptfor the file in flight #1323. Interrupt behaviour was verified by hand on bash 3.2 and bash 5.3, against amainbaseline.