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.
This commit is contained in:
mckero committed 2026-08-28 14:17:50 +01:00
1 parent 23385d2eba
commit 0b879227e5
3 files changed
+89 -7

No files matched your search

+41
View File
@@ -1,5 +1,7 @@
#!/usr/bin/env python3 #!/usr/bin/env python3
"""Unit tests for the QEMU supervisor. Uses a fake launcher, not real QEMU.""" """Unit tests for the QEMU supervisor. Uses a fake launcher, not real QEMU."""
import os
import socket
import unittest import unittest
from uvk5_supervisor import Supervisor from uvk5_supervisor import Supervisor
@@ -196,5 +198,44 @@ class TestSupervisorLogging(unittest.TestCase):
sup.power_off() 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__": if __name__ == "__main__":
unittest.main() unittest.main()
+34 -2
View File
@@ -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. process and killing it is not ours to do.
""" """
import os import os
import socket
import subprocess import subprocess
import threading import threading
import time 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: 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 deadline = time.monotonic() + timeout
while time.monotonic() < deadline: while time.monotonic() < deadline:
if os.path.exists(path): 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) time.sleep(0.05)
return False return False
@@ -96,7 +113,22 @@ class Supervisor:
if self._client is not None: if self._client is not None:
return False return False
self._proc = self._launch() 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 proc = self._proc
self._note("power on") self._note("power on")
# Forward QEMU's own stderr, which run.sh and the tests used to discard. # Forward QEMU's own stderr, which run.sh and the tests used to discard.
+14 -5
View File
@@ -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. # a shared log "who powered it off" is the useful part.
log.add("power", f"{action} requested", ip=client_ip()) log.add("power", f"{action} requested", ip=client_ip())
{"on": supervisor.power_on, try:
"off": supervisor.power_off, {"on": supervisor.power_on,
"reset": supervisor.reset, "off": supervisor.power_off,
"pause": supervisor.pause, "reset": supervisor.reset,
"resume": supervisor.resume}[action]() "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 # 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 # screen, so power off actually goes dark instead of freezing on the last