Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
- A test killed by `--test-timeout` runs its `tear_down`, so a per-test resource is released. Best effort within the watchdog's grace before it sends SIGKILL, so a hook cannot outlive the timeout it cleans up after (#1324)
- A test file that fails to source sweeps its script temp files, so a `bashunit::temp_file` it created at top level no longer survives the run. `bashunit bench` already did this (#1325)
- A malformed `@timeout` or `@retry` runs `tear_down_after_script` before it aborts the run, so the file releases what `set_up_before_script` acquired. Sequential and `--parallel` both leaked it (#1329)
- Ctrl-C releases what an interrupted `--parallel` run acquired: the file's `tear_down_after_script` and the `tear_down` of a test in flight. The worker that owns the file's hook now handles the signal and reaches its test bodies, which a kill from the parent could not (#1331)

## [0.50.0](https://github.com/TypedDevs/bashunit/compare/0.49.0...0.50.0) - 2026-08-18

Expand Down
10 changes: 10 additions & 0 deletions src/runner/discovery.sh
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,21 @@ function bashunit::runner::load_test_files() {
# call_test_functions waits for its own per-test workers before it
# returns, which is what makes this ordering hold.
{
# Ctrl-C reaches this frame as the SIGTERM main::cleanup pkills it with,
# and this is the only frame holding this file's hook (#1331).
trap 'bashunit::runner::cleanup_worker_on_signal' TERM
# A group per test, so the handler above can signal a body subshell
# together with the command it is blocked on. Same reason and same idiom
# as run_with_timeout's watchdog.
set -m
# An aborting annotation returns non-zero here, and this shell runs with
# errexit off, so the hook below still runs. The status is dropped: the
# worker has no channel to abort the parent with, which is a separate
# exit-code defect and not this teardown leak (#1329).
bashunit::runner::call_test_functions "$test_file" "$_cached_fns"
# Settles the debt the parent recorded before dispatch, so a signal that
# lands from here on cannot run the hook a second time.
_BASHUNIT_FILE_TEARDOWN_PENDING=""
bashunit::runner::run_tear_down_after_script "$test_file"
# A hook failure recorded in here dies with the subshell (#1147), so
# publish it the way a test publishes its result.
Expand Down
7 changes: 7 additions & 0 deletions src/runner/exec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ function bashunit::runner::call_test_functions() {
allow_test_parallel=false
fi

# Reset before the first dispatch below: the worker's signal handler group-kills
# every pid recorded here, and a pid left over from the previous file may belong
# to something else by now (#1331).
_BASHUNIT_WORKER_TEST_PIDS=""

# Pre-create the file's result dir before spawning test workers: they all
# publish into it, and checking `[ -d ]` inside a worker races its siblings
# (every worker would still pay the mkdir fork).
Expand All @@ -160,6 +165,7 @@ function bashunit::runner::call_test_functions() {
_test_ordinal=$((_test_ordinal + 1))
_BASHUNIT_RUNNER_RESULT_ORDINAL=$_test_ordinal
bashunit::runner::run_test "$script" "$fn_name" &
_BASHUNIT_WORKER_TEST_PIDS="$_BASHUNIT_WORKER_TEST_PIDS $!"
else
bashunit::runner::run_test "$script" "$fn_name"
fi
Expand Down Expand Up @@ -206,6 +212,7 @@ function bashunit::runner::call_test_functions() {
_test_ordinal=$((_test_ordinal + 1))
_BASHUNIT_RUNNER_RESULT_ORDINAL=$_test_ordinal
bashunit::runner::run_test "$script" "$fn_name" ${parsed_data+"${parsed_data[@]}"} &
_BASHUNIT_WORKER_TEST_PIDS="$_BASHUNIT_WORKER_TEST_PIDS $!"
else
bashunit::runner::run_test "$script" "$fn_name" ${parsed_data+"${parsed_data[@]}"}
fi
Expand Down
51 changes: 51 additions & 0 deletions src/runner/hooks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,57 @@ function bashunit::runner::run_pending_file_teardown() {
bashunit::runner::run_tear_down_after_script "$test_file"
}

# The process groups of the tests this worker still has in flight, empty when it
# has none. Written by call_test_functions as it dispatches, read by the handler
# below, which is the only frame that can reach them.
_BASHUNIT_WORKER_TEST_PIDS=""

##
# The per-file worker's SIGTERM handler.
#
# Since #1320 the worker owns the file's tear_down_after_script, and nothing above
# it can run the hook: several files are in flight under --parallel and the hook is
# unset and redefined as the loop advances, so by interrupt time the parent no
# longer holds the right function body (#1331).
#
# TERM only, and not INT. A shell sets SIGINT to SIG_IGN in a job it backgrounds,
# and a signal ignored on entry can be neither trapped nor reset, so a `trap ...
# INT` in here would be dead code -- measured the same on bash 3.2 and 5.3. Ctrl-C
# reaches this frame as the SIGTERM that main::cleanup pkills it with.
##
function bashunit::runner::cleanup_worker_on_signal() {
# Back to the default disposition first: the hook below is user code and may
# never return, and a handler that cannot itself be interrupted would leave no
# way out but SIGKILL, the same reasoning as main::cleanup.
trap - TERM
# Whole group per test, so the signal reaches the body subshell AND the command
# it is blocked on. Signalling the body alone is not enough: bash defers a trap
# until the running foreground command returns, so a body sitting in the test's
# own `sleep` never reaches the EXIT trap where tear_down lives. The body is a
# great-grandchild of the runner under --parallel, which is why a single
# `pkill -P` from anywhere above cannot do this.
local test_pid
for test_pid in $_BASHUNIT_WORKER_TEST_PIDS; do
kill -TERM -"$test_pid" 2>/dev/null
done
# No `pkill -P $$` sweep to go with the loop. `$$` 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 below. On bash 5 it
# lost that race every time. `$BASHPID` would name the worker, and it is Bash 4+.
#
# A file that opted out of per-test parallelism runs its bodies unforked, so it
# never reaches this handler while a test is running: the worker sits in a
# command substitution, and bash defers a trap until the running foreground
# command returns. Both hooks still run for such a file, when the body finishes
# rather than when the signal lands, so nothing is leaked and the interrupt just
# does not cut that test short.
#
# After the kills, so the per-test tear_down that each body's EXIT trap runs
# comes first, as it does in a normal run.
bashunit::runner::run_pending_file_teardown || true
exit 143
}

function bashunit::runner::run_tear_down_after_script() {
local test_file="$1"
bashunit::internal_log "run_tear_down_after_script"
Expand Down
84 changes: 84 additions & 0 deletions tests/unit/runner/worker_cleanup_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
#!/usr/bin/env bash

# bashunit::runner::cleanup_worker_on_signal is the per-file worker's SIGTERM
# handler. Since #1320 the worker owns the file's tear_down_after_script, and the
# parent cannot run it: several files are in flight under --parallel and the hook
# is unset and redefined as the loop advances, so by interrupt time the parent no
# longer holds the right function body (#1331).
#
# Every call runs inside a subshell: the function ends in `exit`, and kill and
# pkill have to be replaced before they run for real.
#
# No signal is sent here. Delivering one to a real run depends on job control and
# on which frame the shell is in when it lands, which under a loaded --parallel
# suite is not reproducible: an acceptance test doing that failed 2 runs in 3
# while #1323 was being written. These pin the handler's body instead.
#
# Arguments: $1 - order log, $2 - "owed" to record a pending file teardown
#
# The definitions below shadow what the handler calls, so shellcheck sees two
# functions nothing in this file invokes.
# shellcheck disable=SC2329
function _run_worker_cleanup() {
local order=$1
local owed=${2:-}
(
function kill() { printf 'kill %s\n' "$*" >>"$order"; return 0; }
function tear_down_after_script() { printf 'file-teardown\n' >>"$order"; }

_BASHUNIT_WORKER_TEST_PIDS="111 222"

if [ "$owed" = owed ]; then
bashunit::runner::mark_file_teardown_pending "some_test.sh"
fi

bashunit::runner::cleanup_worker_on_signal
) >/dev/null 2>&1 || true
}

function test_the_worker_handler_kills_each_test_group_before_the_file_teardown() {
local order
order="$(bashunit::temp_dir worker_cleanup_owed)/order"

_run_worker_cleanup "$order" owed

# A negative pid signals the whole group, which is what reaches the test body
# subshell and the command it blocks on. A plain per-pid TERM leaves the body a
# live orphan, and a TERM the body defers behind its own foreground command
# never runs its EXIT trap, which is where tear_down lives.
#
# Kills first, so the per-test tear_down each body's EXIT trap runs comes
# before the file hook, as it does in a normal run.
assert_same "kill -TERM -111
kill -TERM -222
file-teardown" "$(cat "$order")"
}

function test_the_worker_handler_runs_no_file_teardown_when_the_file_owes_none() {
local order
order="$(bashunit::temp_dir worker_cleanup_not_owed)/order"

_run_worker_cleanup "$order"

assert_same "kill -TERM -111
kill -TERM -222" "$(cat "$order")"
}

function test_the_worker_handler_runs_the_file_teardown_only_once() {
local order
order="$(bashunit::temp_dir worker_cleanup_twice)/order"

# Settling the debt before the hook runs is what keeps a second delivery, or a
# hook that re-enters this path, from releasing the same resource twice.
(
function kill() { return 0; }
function tear_down_after_script() { printf 'file-teardown\n' >>"$order"; }
function exit() { :; }

bashunit::runner::mark_file_teardown_pending "some_test.sh"
bashunit::runner::cleanup_worker_on_signal
bashunit::runner::cleanup_worker_on_signal
) >/dev/null 2>&1 || true

assert_same "file-teardown" "$(cat "$order")"
}
Loading