Stop cleanup killing unrelated processes, and recover when it happens anyway

Two faults that combined to take the web UI down twice in one session, each time
surfacing to the user as a 502 through the reverse proxy.

`pkill -f 'M uv-k5-v3'` in run.sh and trace_run.sh matched far more than intended.
-f tests the whole command line, so it also matched the shell running the pkill
(the pattern sits in its own argv), any script mentioning the machine type, and the
QEMU child of a running webui.py. Cleanup now lives in tools/lib_kill_emulator.sh:
pgrep -x on the binary name, confirm uv-k5-v3 in /proc/PID/cmdline, and optionally
scope to one QMP socket so a caller only stops the instance it owns.

The supervisor then could not recover from it. power_on() began with
`if self._client is not None: return False`, but a client object is not proof of a
live guest -- after an external kill the stale client made power_on refuse forever,
so the Power button was dead until the whole service was restarted. It now checks
whether the process actually exited and relaunches, logging why.

Tests:

- tools/test_kill_emulator.sh checks a plain process, a process whose command line
  merely mentions uv-k5-v3, and the calling script all survive; that a real emulator
  on a named socket is stopped; that one on another socket is not; and that an
  unscoped call still clears everything. Verified it leaves a live webui.py alone.
- Two supervisor unit tests cover relaunch-after-external-kill and the case that
  must still refuse, so this cannot regress into starting two emulators at once.

Verified end to end against the running web UI: kill the emulator from outside,
status reports unreachable, and pressing Power brings it back to
{"status":"running"} where before it stayed dead.

While writing the first version of the test I modelled the failure as a client
raising BrokenPipeError, which is not what is_running() looks at -- it checks
poll(). The mock was wrong, not the code; the test now has the process report an
exit status, which is what really happens.

tools/lib_kill_emulator.sh

@@ -0,0 +1,47 @@
+#!/usr/bin/env bash
+# Terminate running emulator instances, and nothing else.
+#
+# Source this and call kill_emulators.
+#
+# Why this exists instead of a one-line pkill: `pkill -f 'M uv-k5-v3'` matches the
+# whole command line, which includes
+#
+#   - the shell running the pkill, because the pattern sits in its own argv
+#   - any test script or editor whose command line happens to mention the machine
+#   - a webui.py-owned QEMU, killing the web UI out from under a user; that produced
+#     a 502 through the reverse proxy twice in one session
+#
+# Matching the binary name exactly with pgrep -x, then confirming the machine type
+# in /proc/PID/cmdline, keeps the blast radius to actual emulator processes.
+
+# Kill emulator processes matching a QMP socket path, or all of them if none given.
+#
+# Scoping by socket matters even once the pattern is safe. A test that clears out
+# "any emulator" still kills the one a running webui.py owns: the process survives
+# but its guest is gone, so the user gets a dead screen and
+# {"status":"unreachable"}. Passing the socket the caller is about to use leaves
+# other people's instances alone.
+kill_emulators() {
+    local want_sock="${1:-}" pid cmdline self=$$
+
+    for pid in $(pgrep -x qemu-system-arm 2>/dev/null || true); do
+        [ "$pid" = "$self" ] && continue
+        cmdline=$(tr '\0' ' ' < "/proc/$pid/cmdline" 2>/dev/null) || continue
+
+        # Must be one of ours.
+        case "$cmdline" in
+            *uv-k5-v3*) ;;
+            *) continue ;;
+        esac
+
+        # If a socket was named, only touch the instance serving it.
+        if [ -n "$want_sock" ]; then
+            case "$cmdline" in
+                *"$want_sock"*) ;;
+                *) continue ;;
+            esac
+        fi
+
+        kill "$pid" 2>/dev/null || true
+    done
+}

tools/run.sh

@@ -12,7 +12,11 @@ ELF="${1:-$HOME/uvk5-port/uvk5-sat/build/CW/nr7y.cw.elf}"
 FLASH="$HOME/uvk5-port/sim/assets/flash.img"
 QMP=/tmp/uvk5-qmp.sock
 
-pkill -f 'M uv-k5-v3' 2>/dev/null || true
+# Never `pkill -f 'M uv-k5-v3'` here: see tools/lib_kill_emulator.sh for why that
+# takes down an unrelated webui.py along with it.
+# shellcheck source=tools/lib_kill_emulator.sh
+. "$(dirname "$0")/lib_kill_emulator.sh"
+kill_emulators "$QMP"
 rm -f "$QMP"
 sleep 1
 

tools/test_kill_emulator.sh

@@ -0,0 +1,111 @@
+#!/usr/bin/env bash
+# The cleanup in run.sh must not kill anything other than an emulator.
+#
+# Regression test for a real incident: `pkill -f 'M uv-k5-v3'` matched a running
+# webui.py's QEMU child and the shell issuing the pkill, taking the web UI offline
+# and returning 502 through the reverse proxy. Twice.
+set -u
+
+HERE="$(cd "$(dirname "$0")" && pwd)"
+# shellcheck source=tools/lib_kill_emulator.sh
+. "$HERE/lib_kill_emulator.sh"
+
+fails=0
+note() { printf '  %s\n' "$1"; }
+fail() { printf 'FAIL %s\n' "$1"; fails=$((fails + 1)); }
+
+# A decoy whose command line mentions the machine type but is not the emulator.
+# The old pkill pattern would have killed this.
+sleep 120 &
+decoy=$!
+# Rename via a wrapper so the cmdline contains the trap string.
+bash -c 'exec -a "sleep -M uv-k5-v3 decoy" sleep 120' &
+decoy2=$!
+sleep 1
+
+note "decoy pids: $decoy $decoy2"
+
+kill_emulators
+
+if ! kill -0 "$decoy" 2>/dev/null; then
+    fail "killed a plain sleep"
+else
+    note "PASS  plain process untouched"
+fi
+
+if ! kill -0 "$decoy2" 2>/dev/null; then
+    fail "killed a process merely mentioning uv-k5-v3 in its command line"
+else
+    note "PASS  non-emulator process mentioning uv-k5-v3 untouched"
+fi
+
+# The script running this must survive too; the old pattern matched its own argv.
+note "PASS  this script survived (pid $$)"
+
+kill "$decoy" "$decoy2" 2>/dev/null || true
+wait 2>/dev/null || true
+
+# And a real emulator must actually be terminated.
+QEMU="$HOME/qemu-build/qemu-7.2+dfsg/build/qemu-system-arm"
+ELF="$HOME/uvk5-port/uvk5-sat/build/CW/nr7y.cw.elf"
+IMG=$(mktemp /tmp/kill-test-XXXX.img)
+gzip -dc "$HERE/../assets/pristine/flash-pristine.img.gz" > "$IMG"
+
+IMG2=$(mktemp /tmp/kill-test2-XXXX.img)
+gzip -dc "$HERE/../assets/pristine/flash-pristine.img.gz" > "$IMG2"
+
+if [ -x "$QEMU" ] && [ -f "$ELF" ]; then
+    # Two instances on different sockets, standing in for "the test's own" and
+    # "the one a running web UI owns".
+    "$QEMU" -M "uv-k5-v3,flash-image=$IMG" -nographic -monitor none \
+        -qmp "unix:/tmp/kill-test-a.sock,server=on,wait=off" -kernel "$ELF" \
+        >/dev/null 2>&1 &
+    mine=$!
+    "$QEMU" -M "uv-k5-v3,flash-image=$IMG2" -nographic -monitor none \
+        -qmp "unix:/tmp/kill-test-b.sock,server=on,wait=off" -kernel "$ELF" \
+        >/dev/null 2>&1 &
+    theirs=$!
+    sleep 3
+
+    if ! kill -0 "$mine" 2>/dev/null || ! kill -0 "$theirs" 2>/dev/null; then
+        fail "test emulators did not start"
+    else
+        kill_emulators /tmp/kill-test-a.sock
+        sleep 2
+
+        if kill -0 "$mine" 2>/dev/null; then
+            fail "the targeted emulator was left running"
+        else
+            note "PASS  the emulator on the named socket is terminated"
+        fi
+
+        # This is the property that protects a live session: someone else's
+        # instance must survive a scoped cleanup.
+        if kill -0 "$theirs" 2>/dev/null; then
+            note "PASS  an emulator on another socket survives"
+        else
+            fail "killed an emulator belonging to a different socket"
+        fi
+
+        # Unscoped still clears everything.
+        kill_emulators
+        sleep 2
+        if kill -0 "$theirs" 2>/dev/null; then
+            fail "unscoped cleanup left an emulator running"
+            kill "$theirs" 2>/dev/null || true
+        else
+            note "PASS  unscoped cleanup terminates all emulators"
+        fi
+    fi
+else
+    note "SKIP  emulator binary or firmware missing"
+fi
+
+rm -f "$IMG" "$IMG2" /tmp/kill-test-a.sock /tmp/kill-test-b.sock
+wait 2>/dev/null || true
+
+if [ "$fails" -gt 0 ]; then
+    echo "$fails check(s) failed"
+    exit 1
+fi
+echo "cleanup only ever kills emulators"

tools/test_uvk5_supervisor.py

@@ -3,6 +3,7 @@
 import os
 import socket
 import unittest
+import unittest.mock
 
 from uvk5_supervisor import Supervisor
 
@@ -237,5 +238,58 @@ class TestWaitForSocket(unittest.TestCase):
         self.assertFalse(wait_for_socket(self.path + ".missing", timeout=0.3))
 
 
+class TestRecoversFromAnExternalKill(unittest.TestCase):
+    """Power on must work again after the emulator dies behind the supervisor's back.
+
+    Hit for real, twice: a stray cleanup killed the QEMU that webui.py owned. The
+    supervisor kept its QMP client object, so is_running() reported a broken pipe and
+    power_on() returned False immediately without relaunching -- the Power button was
+    dead until the whole service was restarted.
+    """
+
+    def _supervisor(self, clients):
+        launched = []
+
+        def launch():
+            launched.append(1)
+            proc = unittest.mock.MagicMock()
+            proc.poll.return_value = None
+            proc.stderr = None
+            return proc
+
+        def connect():
+            return clients.pop(0)
+
+        sup = Supervisor(launch, connect)
+        return sup, launched
+
+    def test_power_on_relaunches_when_the_client_is_dead(self):
+        dead = unittest.mock.MagicMock()
+        dead.command.side_effect = BrokenPipeError("dead")
+        fresh = unittest.mock.MagicMock()
+        fresh.command.return_value = {"status": "running"}
+
+        sup, launched = self._supervisor([dead, fresh])
+        self.assertTrue(sup.power_on())
+        self.assertEqual(len(launched), 1)
+
+        # An external kill: the process is gone, so poll() reports an exit status.
+        sup._proc.poll.return_value = -15
+        self.assertFalse(sup.is_running())
+
+        # Power on must notice the client is unusable and start a new emulator.
+        self.assertTrue(sup.power_on(), "power_on refused to relaunch a dead guest")
+        self.assertEqual(len(launched), 2, "no new emulator was launched")
+        self.assertTrue(sup.is_running())
+
+    def test_power_on_still_refuses_when_genuinely_running(self):
+        live = unittest.mock.MagicMock()
+        live.command.return_value = {"status": "running"}
+        sup, launched = self._supervisor([live])
+        self.assertTrue(sup.power_on())
+        self.assertFalse(sup.power_on(), "started a second emulator over a live one")
+        self.assertEqual(len(launched), 1)
+
+
 if __name__ == "__main__":
     unittest.main()

tools/trace_run.sh

@@ -12,7 +12,11 @@ FLASH="$HOME/uvk5-port/sim/assets/flash.img"
 LOG=/tmp/uvk5-trace.log
 QMP=/tmp/uvk5-qmp.sock
 
-pkill -f 'M uv-k5-v3' 2>/dev/null || true
+# Never `pkill -f 'M uv-k5-v3'` here: see tools/lib_kill_emulator.sh for why that
+# takes down an unrelated webui.py along with it.
+# shellcheck source=tools/lib_kill_emulator.sh
+. "$(dirname "$0")/lib_kill_emulator.sh"
+kill_emulators "$QMP"
 rm -f "$QMP" "$LOG"
 sleep 1
 

tools/uvk5_supervisor.py

@@ -110,6 +110,18 @@ class Supervisor:
 
     def power_on(self) -> bool:
         with self._lock:
+            # A client object is not proof of a live guest. If the process died
+            # behind our back -- crashed, OOM-killed, or caught by someone else's
+            # cleanup -- the stale client made this return False forever, so the
+            # Power button did nothing until the whole service was restarted.
+            # Observed twice for real.
+            if self._client is not None and self._proc is not None \
+                    and self._proc.poll() is not None:
+                self._note(f"emulator exited on its own "
+                           f"(status {self._proc.returncode}); restarting")
+                self._client = None
+                self._proc = None
+
             if self._client is not None:
                 return False
             self._proc = self._launch()