From 2b39e6afc42c95bdb859a057e08c5652bb8d4481 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Tue, 10 Nov 2026 10:48:51 -0400 Subject: [PATCH 1/2] bench: the preflight stops its bench on every way out, signalled or not (engine #1668) The load-mode observation runs the bench as a background child, and nothing ended it when the preflight was signalled: the review of #1913 at d098635 showed the child surviving a SIGTERM to the script. The EXIT trap now kills the bench and waits for it before removing the log, and HUP/INT/TERM exit through it. Two arms signal the preflight while a 30 s fake bench runs and assert its pid is gone: with the fix 23 passed, 0 failed; with tools/bench-preflight.sh as at d098635 both arms fail ('bench pid ... is still running'). Co-Authored-By: Claude Fable 5.2 --- tools/bench-preflight.sh | 7 +++++ tools/tests/test-bench-preflight.sh | 46 ++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/tools/bench-preflight.sh b/tools/bench-preflight.sh index ea3fc8789..b3a9d5713 100755 --- a/tools/bench-preflight.sh +++ b/tools/bench-preflight.sh @@ -169,6 +169,13 @@ esac GGML_HRX_LOG_DISPATCH=1 "$bench" -m "$model" -p "$tokens" -n 0 -ngl 99 --device HRX0 -r 3 -lm "$lm" \ >/dev/null 2>"$log" & bench_pid=$! +# The bench is a background child from here on, so nothing ends it when this script ends unless the script does: a +# signalled preflight (a ticket that ends, Ctrl-C, an operator's kill) or a refusal below would otherwise leave it on the +# GPU with no owner. Every way out goes through EXIT, and EXIT stops the bench and waits for it before the log goes. +trap 'kill "$bench_pid" 2>/dev/null; wait "$bench_pid" 2>/dev/null; rm -f "$log"' EXIT +trap 'exit 129' HUP +trap 'exit 130' INT +trap 'exit 143' TERM # The mode is REQUESTED above and OBSERVED here (engine #1668): llama.cpp puts a model it mmaps in `/proc//maps` and a # model it only reads is not there, so the run can be checked against the flag instead of trusted to honour it. A bench that # ignores `-lm` yields a number the header mislabels, which is the same defect as an unstated mode -- one variable measured, diff --git a/tools/tests/test-bench-preflight.sh b/tools/tests/test-bench-preflight.sh index 24bb741d1..43606d871 100755 --- a/tools/tests/test-bench-preflight.sh +++ b/tools/tests/test-bench-preflight.sh @@ -187,7 +187,10 @@ else: open(model, 'rb').read(4096) sys.stderr.write('ggml_hrx dispatch: common.mul_mat.q4_k_q8_1_x4_prefill\n') sys.stderr.write('| qwen3-4b | 2.30 GiB | 4B | HRX | 99 | 1 | pp512 | 3181.97 t/s |\n') -time.sleep(0.7) +sys.stderr.flush() +if os.environ.get('FAKE_BENCH_PIDFILE'): # a case reads this to see whether the bench outlived the preflight + open(os.environ['FAKE_BENCH_PIDFILE'], 'w').write(str(os.getpid())) +time.sleep(float(os.environ.get('FAKE_BENCH_SLEEP') or 0.7)) assert keep is None or len(keep) == 65536 # the mapping must outlive the sleep PY chmod +x "$tmp/mapbench" @@ -217,5 +220,46 @@ else printf 'FAIL %-38s missing reason\n%s\n' "the refusal names its reason" "$(cat "$tmp/lastlm")"; fail=$((fail+1)) fi +# The bench runs in the background while the mode is observed, so the preflight has to take it down on every way out +# (engine #1668, the review of the observation): a preflight that is signalled must not leave a bench behind on the GPU. +# The fake bench sleeps 30 s; "gone" means its pid is no longer a live process shortly after the preflight has ended. +gone() { # gone -> 0 once the process has ended, 1 if it is still running after 3 s + local pid=$1 n=0 + while kill -0 "$pid" 2>/dev/null && ! grep -q '^State:[[:space:]]*Z' "/proc/$pid/status" 2>/dev/null; do + n=$((n + 1)); [ "$n" -gt 30 ] && return 1 + sleep 0.1 + done + return 0 +} +wait_pidfile() { # wait_pidfile -> prints the bench pid once the bench has written it (5 s at most) + local n=0 + until [ -s "$1" ]; do n=$((n + 1)); [ "$n" -gt 50 ] && return 1; sleep 0.1; done + cat "$1" +} +orphan_case() { # orphan_case -> signals the preflight while its bench runs, wants the bench gone + local name=$1 how=$2 root pidfile pre bench + root=$(fake 3600 0 60) + pidfile=$root/bench.pid + rm -f "$pidfile" # fake() hands the same root to equal arguments, so an earlier case's pid would be read + patched "$root" >/dev/null 2>&1 # writes $root/run.sh; without --dispatch it stops after the gates + FAKE_BENCH_SLEEP=30 FAKE_BENCH_PIDFILE=$pidfile \ + PREFLIGHT_MODEL=$tmp/models/big.gguf bash "$root/run.sh" --dispatch "$tmp/mapbench" 512 >/dev/null 2>&1 & + pre=$! + if ! bench=$(wait_pidfile "$pidfile"); then + printf 'FAIL %-38s the fake bench never started\n' "$name"; fail=$((fail+1)) + kill "$pre" 2>/dev/null; return + fi + kill -s "$how" "$pre" 2>/dev/null + wait "$pre" 2>/dev/null + if gone "$bench"; then + printf 'ok %-38s bench pid %s is gone\n' "$name" "$bench"; pass=$((pass+1)) + else + printf 'FAIL %-38s bench pid %s is still running\n' "$name" "$bench"; fail=$((fail+1)) + kill "$bench" 2>/dev/null + fi +} +orphan_case "SIGTERM to the preflight stops the bench" TERM +orphan_case "SIGHUP to the preflight stops the bench" HUP + printf '\nbench-preflight test: %d passed, %d failed\n' "$pass" "$fail" [ "$fail" -eq 0 ] From e122b844a8602cc47b9297322bf437fcf6c8853e Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Tue, 10 Nov 2026 11:45:48 -0400 Subject: [PATCH 2/2] bench: the preflight's stop is bounded, and a reaped bench is not signalled again (review of #1939) The EXIT trap sent TERM and then waited with no deadline: a bench that ignores TERM hung the trap until the bench ended by itself, and a ticket killing the shell in that window leaves the orphan the trap exists to prevent. It now sends TERM, gives the bench at most 5 s (a zombie counts as gone), sends KILL and reaps. bench_pid is emptied once the bench is reaped, so the trap never signals a pid the system has reused, and the trap reads it as ${bench_pid:-}. New arm: a fake bench that ignores SIGTERM. With the fix 24 passed, 0 failed; with the script as at 2b39e6a the arm fails ('the preflight took 30s to return'). Co-Authored-By: Claude Fable 5.2 --- tools/bench-preflight.sh | 25 ++++++++++++++++++++++--- tools/tests/test-bench-preflight.sh | 18 ++++++++++++++---- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/tools/bench-preflight.sh b/tools/bench-preflight.sh index b3a9d5713..1ec38755d 100755 --- a/tools/bench-preflight.sh +++ b/tools/bench-preflight.sh @@ -172,7 +172,24 @@ bench_pid=$! # The bench is a background child from here on, so nothing ends it when this script ends unless the script does: a # signalled preflight (a ticket that ends, Ctrl-C, an operator's kill) or a refusal below would otherwise leave it on the # GPU with no owner. Every way out goes through EXIT, and EXIT stops the bench and waits for it before the log goes. -trap 'kill "$bench_pid" 2>/dev/null; wait "$bench_pid" 2>/dev/null; rm -f "$log"' EXIT +# TERM first, at most 5 s for the bench to go, then KILL, then reap: a bench that ignores TERM (wedged in the device) must +# not hang this trap, because a ticket that then kills the shell leaves the same orphan. A child that has exited stays a +# zombie until it is waited for and still answers `kill -0`, so "gone" is read from its state. bench_pid is emptied once +# the bench is reaped, here or below, so a pid the system has handed to another process is never signalled. +stop_bench() { + [ -n "${bench_pid:-}" ] || return 0 + kill "$bench_pid" 2>/dev/null + local n=0 + while [ "$n" -lt 50 ] && kill -0 "$bench_pid" 2>/dev/null && + ! grep -q '^State:[[:space:]]*Z' "/proc/$bench_pid/status" 2>/dev/null; do + sleep 0.1 + n=$((n + 1)) + done + kill -KILL "$bench_pid" 2>/dev/null + wait "$bench_pid" 2>/dev/null + bench_pid= +} +trap 'stop_bench; rm -f "$log"' EXIT trap 'exit 129' HUP trap 'exit 130' INT trap 'exit 143' TERM @@ -189,6 +206,8 @@ while :; do done bench_rc=0 wait "$bench_pid" || bench_rc=$? +bench_was=$bench_pid # for the messages below; the process is reaped, so the trap has nothing left to stop +bench_pid= [ "$bench_rc" -eq 0 ] || fatal "the bench exited $bench_rc; see $log" # Only the modes whose mapping is unambiguous are asserted: `mmap`/`mmap+mlock` map, `none`/`dio` do not, and `mlock` alone is # not asserted because whether it maps depends on the loader -- a gate that guessed would fail a correct run. An OBSERVED @@ -199,9 +218,9 @@ if [ "$mapped" = no ] && [ "$alive" -eq 0 ]; then else case "$lm:$mapped" in mmap:no|mmap+mlock:no) - fatal "the load mode is -lm $lm but the model is not in /proc/$bench_pid/maps; the run is not the measurement the header claims (engine #1668)" ;; + fatal "the load mode is -lm $lm but the model is not in /proc/$bench_was/maps; the run is not the measurement the header claims (engine #1668)" ;; none:yes|dio:yes) - fatal "the load mode is -lm $lm but the model IS in /proc/$bench_pid/maps; the run is not the measurement the header claims (engine #1668)" ;; + fatal "the load mode is -lm $lm but the model IS in /proc/$bench_was/maps; the run is not the measurement the header claims (engine #1668)" ;; esac fi diff --git a/tools/tests/test-bench-preflight.sh b/tools/tests/test-bench-preflight.sh index 43606d871..ead5e1f7d 100755 --- a/tools/tests/test-bench-preflight.sh +++ b/tools/tests/test-bench-preflight.sh @@ -174,7 +174,7 @@ fi head -c 65536 /dev/urandom > "$tmp/models/big.gguf" # non-empty: mmap of a zero-length file fails cat > "$tmp/mapbench" <<'PY' #!/usr/bin/env python3 -import mmap, os, sys, time +import mmap, os, signal, sys, time a = sys.argv[1:] lm = a[a.index('-lm') + 1] if '-lm' in a else 'mmap' model = a[a.index('-m') + 1] @@ -188,6 +188,8 @@ else: sys.stderr.write('ggml_hrx dispatch: common.mul_mat.q4_k_q8_1_x4_prefill\n') sys.stderr.write('| qwen3-4b | 2.30 GiB | 4B | HRX | 99 | 1 | pp512 | 3181.97 t/s |\n') sys.stderr.flush() +if os.environ.get('FAKE_BENCH_IGNORE_TERM'): # a bench wedged in the device does not answer SIGTERM + signal.signal(signal.SIGTERM, signal.SIG_IGN) if os.environ.get('FAKE_BENCH_PIDFILE'): # a case reads this to see whether the bench outlived the preflight open(os.environ['FAKE_BENCH_PIDFILE'], 'w').write(str(os.getpid())) time.sleep(float(os.environ.get('FAKE_BENCH_SLEEP') or 0.7)) @@ -236,21 +238,28 @@ wait_pidfile() { # wait_pidfile -> prints the bench pid once the bench h until [ -s "$1" ]; do n=$((n + 1)); [ "$n" -gt 50 ] && return 1; sleep 0.1; done cat "$1" } -orphan_case() { # orphan_case -> signals the preflight while its bench runs, wants the bench gone - local name=$1 how=$2 root pidfile pre bench +orphan_case() { # orphan_case [ignore-term] -> signals the preflight while its bench runs, wants the bench gone + local name=$1 how=$2 ignore=${3:-} root pidfile pre bench started root=$(fake 3600 0 60) pidfile=$root/bench.pid rm -f "$pidfile" # fake() hands the same root to equal arguments, so an earlier case's pid would be read patched "$root" >/dev/null 2>&1 # writes $root/run.sh; without --dispatch it stops after the gates - FAKE_BENCH_SLEEP=30 FAKE_BENCH_PIDFILE=$pidfile \ + FAKE_BENCH_SLEEP=30 FAKE_BENCH_PIDFILE=$pidfile FAKE_BENCH_IGNORE_TERM=$ignore \ PREFLIGHT_MODEL=$tmp/models/big.gguf bash "$root/run.sh" --dispatch "$tmp/mapbench" 512 >/dev/null 2>&1 & pre=$! if ! bench=$(wait_pidfile "$pidfile"); then printf 'FAIL %-38s the fake bench never started\n' "$name"; fail=$((fail+1)) kill "$pre" 2>/dev/null; return fi + started=$SECONDS kill -s "$how" "$pre" 2>/dev/null wait "$pre" 2>/dev/null + # The bench sleeps 30 s. A preflight that only returns once the bench ends by itself has not stopped it, it has waited + # for it - which is what an unbounded wait on a bench that ignores TERM does. + if [ $((SECONDS - started)) -ge 15 ]; then + printf 'FAIL %-38s the preflight took %ds to return\n' "$name" "$((SECONDS - started))"; fail=$((fail+1)) + kill -KILL "$bench" 2>/dev/null; return + fi if gone "$bench"; then printf 'ok %-38s bench pid %s is gone\n' "$name" "$bench"; pass=$((pass+1)) else @@ -260,6 +269,7 @@ orphan_case() { # orphan_case -> signals the preflight while it } orphan_case "SIGTERM to the preflight stops the bench" TERM orphan_case "SIGHUP to the preflight stops the bench" HUP +orphan_case "a bench that ignores TERM is killed" TERM 1 printf '\nbench-preflight test: %d passed, %d failed\n' "$pass" "$fail" [ "$fail" -eq 0 ]