From 0b879227e54fcd8fb3788b3edb151fe9ce8ba646 Mon Sep 17 00:00:00 2001 From: MCKero Date: Fri, 28 Aug 2026 14:17:50 +0100 Subject: [PATCH] Do not mistake a leftover socket file for a running emulator Power on returned HTTP 500 with a ConnectionRefusedError traceback. A unix socket file outlives the process that created it, so a killed QEMU left /tmp/uvk5-qmp.sock behind; wait_for_socket only checked os.path.exists, returned immediately, and the connect then failed. It now probes with a real connect, which distinguishes "listening" from "leftover file". Two related hardenings: - power_on cleans up if connecting fails. Otherwise a half-started QEMU keeps running untracked, holds the socket, and blocks the next power on -- which is how one stale socket turned into a repeatable failure. - The route reports a failed power action as 503 with the reason, instead of a 500 and a traceback the browser cannot display. Three tests cover the stale socket, a real listener, and a path that never appears. --- tools/test_uvk5_supervisor.py | 41 +++++++++++++++++++++++++++++++++++ tools/uvk5_supervisor.py | 36 ++++++++++++++++++++++++++++-- tools/webui.py | 19 +++++++++++----- 3 files changed, 89 insertions(+), 7 deletions(-) diff --git a/tools/test_uvk5_supervisor.py b/tools/test_uvk5_supervisor.py index 0d1fdb9..eb643fa 100644 --- a/tools/test_uvk5_supervisor.py +++ b/tools/test_uvk5_supervisor.py @@ -1,5 +1,7 @@ #!/usr/bin/env python3 """Unit tests for the QEMU supervisor. Uses a fake launcher, not real QEMU.""" +import os +import socket import unittest from uvk5_supervisor import Supervisor @@ -196,5 +198,44 @@ class TestSupervisorLogging(unittest.TestCase): sup.power_off() +class TestWaitForSocket(unittest.TestCase): + """A leftover socket file must not be mistaken for a listening emulator. + + Hit for real: a killed QEMU left /tmp/uvk5-qmp.sock behind, wait_for_socket + returned immediately because the path existed, and the connect then failed with + ECONNREFUSED -- which the user saw as power on returning HTTP 500. + """ + + def setUp(self): + import tempfile + self.dir = tempfile.mkdtemp() + self.path = os.path.join(self.dir, "qmp.sock") + + def tearDown(self): + import shutil + shutil.rmtree(self.dir, ignore_errors=True) + + def test_returns_false_for_a_stale_socket_file(self): + from uvk5_supervisor import wait_for_socket + # A socket file with nothing listening: bind then close. + srv = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + srv.bind(self.path) + srv.close() + self.assertTrue(os.path.exists(self.path), "need a leftover file") + self.assertFalse(wait_for_socket(self.path, timeout=0.5)) + + def test_returns_true_when_something_is_listening(self): + from uvk5_supervisor import wait_for_socket + srv = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + srv.bind(self.path) + srv.listen(1) + self.addCleanup(srv.close) + self.assertTrue(wait_for_socket(self.path, timeout=2)) + + def test_returns_false_when_the_path_never_appears(self): + from uvk5_supervisor import wait_for_socket + self.assertFalse(wait_for_socket(self.path + ".missing", timeout=0.3)) + + if __name__ == "__main__": unittest.main() diff --git a/tools/uvk5_supervisor.py b/tools/uvk5_supervisor.py index efd0024..a28edb8 100644 --- a/tools/uvk5_supervisor.py +++ b/tools/uvk5_supervisor.py @@ -14,6 +14,7 @@ started with run.sh. Then `power_off` must refuse, because we did not start that process and killing it is not ours to do. """ import os +import socket import subprocess import threading import time @@ -41,10 +42,26 @@ def default_launcher(qemu: str, flash: str, elf: str, def wait_for_socket(path: str, timeout: float = 15.0) -> bool: + """Wait until something is actually accepting connections on `path`. + + Existence is not enough. A unix socket file outlives the process that created + it, so a crashed or killed emulator leaves one behind, and a plain + os.path.exists() check returns immediately and the connect then fails with + ECONNREFUSED -- which surfaces as power on returning 500. Probing with a real + connect distinguishes "listening" from "leftover file". + """ deadline = time.monotonic() + timeout while time.monotonic() < deadline: if os.path.exists(path): - return True + probe = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + try: + probe.settimeout(1.0) + probe.connect(path) + return True + except OSError: + pass # stale, or not listening yet + finally: + probe.close() time.sleep(0.05) return False @@ -96,7 +113,22 @@ class Supervisor: if self._client is not None: return False self._proc = self._launch() - self._client = self._connect() + try: + self._client = self._connect() + except Exception as exc: + # Do not leave a half-started emulator behind: the process would + # keep running with no client tracking it, hold the QMP socket, and + # block the next power on. Clean up and report instead. + proc, self._proc = self._proc, None + self._client = None + if proc is not None: + proc.terminate() + try: + proc.wait(timeout=5) + except Exception: + proc.kill() + self._note(f"power on failed: {exc}") + raise proc = self._proc self._note("power on") # Forward QEMU's own stderr, which run.sh and the tests used to discard. diff --git a/tools/webui.py b/tools/webui.py index 0654ff2..2b0088f 100644 --- a/tools/webui.py +++ b/tools/webui.py @@ -197,11 +197,20 @@ def create_app(client, frame_addr: int, status_addr: int, scale: int = 4, # a shared log "who powered it off" is the useful part. log.add("power", f"{action} requested", ip=client_ip()) - {"on": supervisor.power_on, - "off": supervisor.power_off, - "reset": supervisor.reset, - "pause": supervisor.pause, - "resume": supervisor.resume}[action]() + try: + {"on": supervisor.power_on, + "off": supervisor.power_off, + "reset": supervisor.reset, + "pause": supervisor.pause, + "resume": supervisor.resume}[action]() + except Exception as exc: + # Starting the emulator can genuinely fail -- a stale QMP socket, a + # missing binary, a port already taken. Report it as a failed action + # rather than a 500 with a traceback the browser cannot show. + log.add("power", f"{action} failed: {exc}", ip=client_ip()) + pump.rebind(supervisor.client()) + return jsonify(error=f"{action} failed: {exc}", + powered=supervisor.is_running()), 503 # Point the pump at whatever client is live now. rebind(None) blanks the # screen, so power off actually goes dark instead of freezing on the last