Skip to content

fix(runner): release both teardowns when Ctrl-C stops a parallel run - #1336

Merged
Chemaclass merged 2 commits into
mainfrom
fix/1331-ctrl-c-runs-neither-teardown-under-parallel
Aug 21, 2026
Merged

fix(runner): release both teardowns when Ctrl-C stops a parallel run#1336
Chemaclass merged 2 commits into
mainfrom
fix/1331-ctrl-c-runs-neither-teardown-under-parallel

Conversation

@Chemaclass

@Chemaclass Chemaclass commented Aug 21, 2026

Copy link
Copy Markdown
Member

🤔 Background

Related #1331

Under --parallel, Ctrl-C released neither the per-test tear_down nor the file's tear_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

  • The worker handles the signal itself and settles the teardown debt the parent recorded before dispatch. The debt is cleared before the hook runs, so a signal landing mid-teardown cannot release the same resource twice.
  • Each test gets its own process group, and the handler signals the group. A per-pid signal is not enough: bash defers a trap until the running command returns, so a body sitting in the test's own sleep never reaches the EXIT trap where tear_down lives.
  • TERM only. A terminal does deliver Ctrl-C's INT to the whole foreground group, worker included, but the worker never sees it: a shell sets SIGINT to SIG_IGN in 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.
  • Two other corrections to the issue's model: the test body is a great-grandchild of the runner, not a grandchild, and no pkill -P $$ sweep is possible in the worker, because $$ names the runner inside a subshell and the sweep killed the handler itself.
  • Known limit, measured: a file opting out of per-test parallelism runs its bodies unforked, so the worker sits in a command substitution and defers the trap. Both hooks still run for such a file, when the body finishes rather than when the signal lands. Nothing is leaked; the interrupt just does not cut that test short.
  • Unit-tested rather than signal-tested, as the issue prescribes: a real delivery depends on which frame the shell is in and flaked 2 runs in 3 for Ctrl-C does not run tear_down_after_script for the file in flight #1323. Interrupt behaviour was verified by hand on bash 3.2 and bash 5.3, against a main baseline.

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
@Chemaclass Chemaclass added the bug Something isn't working label Aug 21, 2026
@Chemaclass Chemaclass self-assigned this Aug 21, 2026
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
Chemaclass merged commit 4c5c99f into main Aug 21, 2026
37 checks passed
@Chemaclass
Chemaclass deleted the fix/1331-ctrl-c-runs-neither-teardown-under-parallel branch August 21, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant