diff --git a/bin/fm-timeout-lib.sh b/bin/fm-timeout-lib.sh index a785ad8b793..cb9c58b6f0d 100644 --- a/bin/fm-timeout-lib.sh +++ b/bin/fm-timeout-lib.sh @@ -24,19 +24,20 @@ # command chose to die. # # fm_exec_timed [args...] -# Replaces the calling shell with the bounded command, so it must be the -# last command of a subshell: the bound kills the command, not the -# caller. The command runs in its own process group; TERM goes to that -# group at the bound, and KILL once more have passed, +# Replaces the calling shell with the watchdog, so call it as the final +# command in the shell or subshell to be replaced. The command runs in +# its own process group; TERM goes to that group at the bound, and KILL +# once more have passed, # for a command that ignores TERM or is mid-way through work it will not # abandon. A TERM, INT, or HUP delivered to the bounding process is # forwarded to the group and starts the same grace. The perl watchdog # also starts that escalation when its own parent dies before it could # be signalled (an owner torn down by an outer group-kill cannot leave -# the bounded subtree orphaned behind it). The owner is captured before -# the watchdog starts: FM_EXEC_TIMED_OWNER_PID when the caller names it, -# else the calling script ($$) when fm_exec_timed runs in a subshell, -# else the shell's parent. The escalation starts once that owner is gone +# the bounded subtree orphaned behind it). The shell passes an owner +# candidate to the watchdog: FM_EXEC_TIMED_OWNER_PID when the caller +# names it, else $$. When that candidate is the watchdog's own pid +# because the calling shell was replaced, the watchdog uses the shell's +# parent instead. The escalation starts once that owner is gone # or the watchdog's parent changes, so an owner that dies while the # watchdog is still starting is detected too. The timeout/gtimeout # fallback does not track the owner: it bounds the command only by its @@ -220,12 +221,16 @@ fm_exec_timed() { # echo "fm_exec_timed: usage: fm_exec_timed [args...]" >&2 exit 125 fi + # The watchdog resolves an owner that is its own pid - the calling shell + # itself rather than a subshell, which the exec replaces - to that shell's + # parent. It decides this from its own pid because $BASHPID, which would + # tell a subshell from the calling shell here, does not exist in bash 3.2. owner=${FM_EXEC_TIMED_OWNER_PID:-$$} - [ "$owner" != "$BASHPID" ] || owner=$PPID unset FM_EXEC_TIMED_OWNER_PID if command -v perl >/dev/null 2>&1; then exec perl -MPOSIX=WNOHANG,setpgid -MTime::HiRes=time -e ' - my ($bound, $grace, $owner) = (shift, shift, shift); + my ($bound, $grace, $owner, $caller_parent) = (shift, shift, shift, shift); + $owner = $caller_parent if $owner == $$; my $parent = getppid(); my ($pid, $pending, $kill_at, $timed_out) = (0, "", 0, 0); for my $sig (qw(TERM INT HUP)) { @@ -272,7 +277,7 @@ fm_exec_timed() { # } select undef, undef, undef, 0.05; } - ' -- "$seconds" "$grace" "$owner" "$@" + ' -- "$seconds" "$grace" "$owner" "$PPID" "$@" elif command -v timeout >/dev/null 2>&1; then exec timeout -k "$grace" "$seconds" "$@" elif command -v gtimeout >/dev/null 2>&1; then diff --git a/tests/fm-timeout-lib.test.sh b/tests/fm-timeout-lib.test.sh index 56bcc6dfc5a..9606872934a 100755 --- a/tests/fm-timeout-lib.test.sh +++ b/tests/fm-timeout-lib.test.sh @@ -109,7 +109,7 @@ test_the_bound_replaces_the_calling_shell() { rm -f "$dir/caller" "$dir/parent" ( . "$ROOT/bin/fm-timeout-lib.sh" - printf '%s\n' "$BASHPID" > "$dir/caller" + bash -c 'printf "%s\n" "$PPID" > "$1"' _ "$dir/caller" PATH=$path fm_exec_timed 5 1 bash -c 'echo "$PPID" > "$1"' _ "$dir/parent" ) || fail "the bounded probe failed under PATH=$path" caller=$(cat "$dir/caller") @@ -172,6 +172,13 @@ test_a_signal_to_the_bounding_process_reaches_the_command() { pass "fm_exec_timed forwards a TERM it receives to the bounded command" } +# Stock macOS /bin/bash is 3.2, which has no $BASHPID; the cases that name it +# also run under it where it exists. +STOCK_BASH32= +if [ -x /bin/bash ] && [ "$(/bin/bash -c 'printf "%s.%s" "${BASH_VERSINFO[0]}" "${BASH_VERSINFO[1]}"')" = 3.2 ]; then + STOCK_BASH32=/bin/bash +fi + # A caller that names its owner before launching the watchdog is watched even # when that owner died while the watchdog was still starting: the watchdog's # parent is then not the named owner, so the escalation starts at once rather @@ -204,32 +211,89 @@ test_a_named_owner_that_is_gone_ends_the_command() { # fm_exec_timed - the watchdog then starts already reparented - is still # detected instead of leaving the command running to its bound. test_an_owner_that_dies_during_startup_ends_the_command() { - local dir watchdog started - dir="$TMP_ROOT/startup-owner" - mkdir -p "$dir" - # shellcheck disable=SC2016 - PATH=$PERL_ONLY bash -c ' - . "$1/bin/fm-timeout-lib.sh" - ( - echo "$BASHPID" > "$2/watchdog" - while kill -0 "$$" 2>/dev/null; do sleep 0.05; done - fm_exec_timed 60 1 bash -c "exec sleep 300" - ) >/dev/null 2>&1 & - exit 0 - ' _ "$ROOT" "$dir" - wait_for_file "$dir/watchdog" - watchdog=$(cat "$dir/watchdog") - started=$SECONDS - while kill -0 "$watchdog" 2>/dev/null; do - if [ "$((SECONDS - started))" -ge 15 ]; then - kill -KILL "$watchdog" 2>/dev/null || true - fail "a watchdog whose owner died during startup ran on toward its bound" - fi - sleep 0.02 + local shell dir watchdog started + for shell in bash $STOCK_BASH32; do + dir="$TMP_ROOT/startup-owner-${shell//\//_}" + mkdir -p "$dir" + # The subshell's own pid comes from a child's $PPID, since bash 3.2 has no + # $BASHPID. + # shellcheck disable=SC2016 + PATH=$PERL_ONLY "$shell" -c ' + set -u + . "$1/bin/fm-timeout-lib.sh" + ( + bash -c "echo \"\$PPID\"" > "$2/watchdog" + while kill -0 "$$" 2>/dev/null; do sleep 0.05; done + fm_exec_timed 60 1 bash -c "exec sleep 300" + ) >/dev/null 2>&1 & + exit 0 + ' _ "$ROOT" "$dir" + wait_for_file "$dir/watchdog" + watchdog=$(cat "$dir/watchdog") + started=$SECONDS + while kill -0 "$watchdog" 2>/dev/null; do + if [ "$((SECONDS - started))" -ge 15 ]; then + kill -KILL "$watchdog" 2>/dev/null || true + fail "under $shell, a watchdog whose owner died during startup ran on toward its bound" + fi + sleep 0.02 + done done pass "fm_exec_timed ends the command when its owner dies during watchdog startup" } +# A stock-bash caller under set -u runs the bounded command and gets its +# status back, rather than dying on a variable bash 3.2 does not define. +test_stock_bash32_runs_the_bounded_command() { + local out rc=0 + if [ -z "$STOCK_BASH32" ]; then + printf 'skip - stock Bash 3.2 is unavailable\n' + return 0 + fi + out=$("$STOCK_BASH32" -c ' + set -u + . "$1/bin/fm-timeout-lib.sh" + PATH=$2 fm_exec_timed 5 1 bash -c "echo ran; exit 7" + ' _ "$ROOT" "$PERL_ONLY" 2>&1) || rc=$? + [ "$rc" -eq 7 ] || fail "stock Bash 3.2 did not pass the command's status through (rc=$rc): $out" + [ "$out" = ran ] || fail "stock Bash 3.2 printed '$out' instead of the command's output" + pass "fm_exec_timed runs the bounded command under stock Bash 3.2 with set -u" +} + +# Called directly from a script rather than a subshell, the exec replaces the +# script itself, so the owner is the script's parent. A parent that dies while +# the watchdog is still starting - the watchdog then starts already +# reparented - is detected instead of leaving the command running to its +# bound, on every bash the host has, including one with no $BASHPID. +test_a_direct_callers_parent_that_dies_during_startup_ends_the_command() { + local shell dir watchdog started + for shell in bash $STOCK_BASH32; do + dir="$TMP_ROOT/direct-owner-${shell//\//_}" + mkdir -p "$dir" + # shellcheck disable=SC2016 + printf '%s\n' \ + 'set -u' \ + '. "$1/bin/fm-timeout-lib.sh"' \ + 'echo "$$" > "$2/watchdog"' \ + 'while kill -0 "$PPID" 2>/dev/null; do sleep 0.05; done' \ + 'fm_exec_timed 60 1 bash -c "exec sleep 300"' > "$dir/caller.sh" + # shellcheck disable=SC2016 + PATH=$PERL_ONLY "$shell" -c '"$0" "$1" "$2" "$3" >/dev/null 2>&1 & exit 0' \ + "$shell" "$dir/caller.sh" "$ROOT" "$dir" + wait_for_file "$dir/watchdog" + watchdog=$(cat "$dir/watchdog") + started=$SECONDS + while kill -0 "$watchdog" 2>/dev/null; do + if [ "$((SECONDS - started))" -ge 15 ]; then + kill -KILL "$watchdog" 2>/dev/null || true + fail "under $shell, a watchdog whose direct caller's parent died during startup ran on toward its bound" + fi + sleep 0.02 + done + done + pass "fm_exec_timed ends the command when a direct caller's parent dies during watchdog startup" +} + # perl is preferred whenever it exists, because only its watchdog can reap a # leftover descendant after replacing the caller. test_perl_is_preferred_over_timeout() { @@ -337,6 +401,8 @@ test_a_descendant_holding_the_output_cannot_outlast_the_bound test_a_signal_to_the_bounding_process_reaches_the_command test_a_named_owner_that_is_gone_ends_the_command test_an_owner_that_dies_during_startup_ends_the_command +test_stock_bash32_runs_the_bounded_command +test_a_direct_callers_parent_that_dies_during_startup_ends_the_command test_perl_is_preferred_over_timeout test_refuses_rather_than_running_unbounded test_rejects_malformed_bounds_before_running_anything