bench: the preflight stops its bench when it is signalled (answers the CHANGES on #1913, engine #1668)

tools/bench-preflight.sh

@@ -169,6 +169,30 @@ 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.
+# 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
 # The mode is REQUESTED above and OBSERVED here (engine #1668): llama.cpp puts a model it mmaps in `/proc/<pid>/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,
@@ -182,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
@@ -192,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
 

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]
@@ -187,7 +187,12 @@ 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_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))
 assert keep is None or len(keep) == 65536          # the mapping must outlive the sleep
 PY
 chmod +x "$tmp/mapbench"
@@ -217,5 +222,54 @@ 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 <pid> -> 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 <file> -> 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 <name> <signal> [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_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
+        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
+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 ]