From e89a57bb97b5a0d624e8d490b6b8aa38ba140817 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Sat, 5 Sep 2026 13:06:55 +0200 Subject: fix(win): graceful shutdown, one startup-mode control, and a stray-\r bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows-only changes, all found by actually running the previous session's work rather than by review alone: - CTRL_CLOSE_EVENT/LOGOFF/SHUTDOWN handler (platform.py, ctypes SetConsoleCtrlHandler) so closing a console window, signing off, or a system shutdown runs the daemon's real _shutdown() instead of Windows just ending the process — closing WebRTC sessions and any in-flight ffmpeg transcode instead of orphaning it. `taskkill /F` itself stays uncatchable (like SIGKILL), so autostart_run() now spawns with CREATE_NEW_PROCESS_GROUP instead of DETACHED_PROCESS and autostart_end() tries CTRL_BREAK_EVENT against the recorded pid first, falling back to the hard kill only if that doesn't stop it in time. - Replaced the Node page's two independent autostart/service-mode toggles with one "start automatically" select (off / at sign-in / as a background service). The old pair let both be active at once — starting the daemon twice, at boot and at sign-in — and their layout broke wrapping inside .node-service's flex row. The new control always removes whichever mechanism is active before installing the target; platform.py's service_install() does the same on the CLI side. The "background service" option disables itself (with a hint pointing at the CLI) when running unpackaged, since service-mode.ps1/service.ps1/firewall.ps1 all assume an installed build's layout — verified live rather than assumed by actually running those scripts unelevated. - findNodeBinary() no longer bakes a stray \r into resolved paths. Found by rebooting after enabling per-user autostart: where.exe listed two matches, and stdout.trim().split('\n')[0] only strips the whole string's ends, leaving line one's own trailing \r attached — which landed inside the Startup .vbs's quoted path and broke it with "Unterminated string constant" at boot. Fixed by splitting on \r?\n and trimming every line. - Dependency audit for the Windows installer (docs/WINDOWS-PORT.md): no VC++ Redistributable needed, confirmed by inspecting the built node-runtime's actual import table rather than assuming. New docs/windows-build.md: a concise clone-to-installer build guide. Co-Authored-By: Claude Sonnet 5 --- packages/meshbay-node/tests/test_platform.py | 192 ++++++++++++++++++++++++++- 1 file changed, 188 insertions(+), 4 deletions(-) (limited to 'packages/meshbay-node/tests/test_platform.py') diff --git a/packages/meshbay-node/tests/test_platform.py b/packages/meshbay-node/tests/test_platform.py index 7042afb..91713be 100644 --- a/packages/meshbay-node/tests/test_platform.py +++ b/packages/meshbay-node/tests/test_platform.py @@ -164,14 +164,26 @@ def test_autostart_install_refuses_off_windows(monkeypatch): plat.autostart_install(exe="/usr/bin/meshbay-node") -def test_autostart_run_launches_the_resolved_exe_detached(win_startup, monkeypatch): +def test_autostart_run_launches_the_resolved_exe_windowless_and_records_its_pid( + win_startup, monkeypatch, tmp_path): monkeypatch.setattr(plat, "_node_exe", lambda: r"C:\x\meshbay-node.exe") + monkeypatch.setenv("LOCALAPPDATA", str(tmp_path)) # state_dir() -> pidfile location calls = {} - monkeypatch.setattr(plat.subprocess, "Popen", - lambda argv, **kw: calls.update(argv=argv, kw=kw)) + + def fake_popen(argv, **kw): + calls.update(argv=argv, kw=kw) + return Mock(pid=4242) + + monkeypatch.setattr(plat.subprocess, "Popen", fake_popen) plat.autostart_run() assert calls["argv"] == [r"C:\x\meshbay-node.exe"] - assert calls["kw"]["creationflags"] & 0x08000000 # CREATE_NO_WINDOW + flags = calls["kw"]["creationflags"] + assert flags & 0x08000000 # CREATE_NO_WINDOW + assert flags & 0x00000200 # CREATE_NEW_PROCESS_GROUP + assert not flags & 0x00000008 # not DETACHED_PROCESS -- that has no + # console at all, so CTRL_BREAK_EVENT + # would have nothing to signal + assert plat._pid_file().read_text(encoding="utf-8") == "4242" def test_autostart_run_refuses_off_windows(monkeypatch): @@ -180,6 +192,178 @@ def test_autostart_run_refuses_off_windows(monkeypatch): plat.autostart_run() +# ── Graceful stop (CTRL_BREAK_EVENT + taskkill fallback) ──────────────────── +# +# autostart_end() references signal.CTRL_BREAK_EVENT, which genuinely does not +# exist in the `signal` module off Windows -- monkeypatching sys.platform +# cannot manufacture it, unlike the pure-Python behaviour tested above. Skip +# rather than mock around it, matching test_configure_event_loop_selector_opt_in. + +@pytest.mark.skipif(sys.platform != "win32", + reason="signal.CTRL_BREAK_EVENT exists only on win32") +def test_autostart_end_stops_gracefully_when_ctrl_break_is_enough(monkeypatch, tmp_path): + monkeypatch.setattr(sys, "platform", "win32") + monkeypatch.setenv("LOCALAPPDATA", str(tmp_path)) + plat._pid_file().parent.mkdir(parents=True, exist_ok=True) + plat._pid_file().write_text("4242", encoding="utf-8") + + kill_calls = [] + monkeypatch.setattr(plat.os, "kill", lambda pid, sig: kill_calls.append((pid, sig))) + # Alive (our exe) on the pre-signal check, gone by the first poll after -- + # a plain constant can't tell those two calls apart. + seen = {"n": 0} + + def fake_check(pid): + seen["n"] += 1 + return seen["n"] == 1 + + monkeypatch.setattr(plat, "_pid_is_meshbay_node", fake_check) + run_calls = [] + monkeypatch.setattr(plat.subprocess, "run", + lambda argv, **kw: run_calls.append(argv)) + + plat.autostart_end() + + assert kill_calls == [(4242, plat.signal.CTRL_BREAK_EVENT)] + assert run_calls == [] # no taskkill needed + assert not plat._pid_file().exists() + + +@pytest.mark.skipif(sys.platform != "win32", + reason="signal.CTRL_BREAK_EVENT exists only on win32") +def test_autostart_end_falls_back_to_taskkill_when_the_pid_never_exits( + monkeypatch, tmp_path): + monkeypatch.setattr(sys, "platform", "win32") + monkeypatch.setenv("LOCALAPPDATA", str(tmp_path)) + plat._pid_file().parent.mkdir(parents=True, exist_ok=True) + plat._pid_file().write_text("4242", encoding="utf-8") + + monkeypatch.setattr(plat.os, "kill", lambda pid, sig: None) + monkeypatch.setattr(plat, "_pid_is_meshbay_node", lambda pid: True) # never exits + monkeypatch.setattr(plat.time, "sleep", lambda s: None) # don't really wait + clock = iter([0.0, 1.0, 6.0]) # deadline = 0.0 + 5.0; third read is past it + monkeypatch.setattr(plat.time, "monotonic", lambda: next(clock)) + run_calls = [] + monkeypatch.setattr(plat.subprocess, "run", + lambda argv, **kw: run_calls.append(argv)) + + plat.autostart_end() + + assert run_calls == [["taskkill", "/IM", "meshbay-node.exe", "/F"]] + assert not plat._pid_file().exists() + + +def test_autostart_end_falls_back_to_taskkill_without_a_pidfile(monkeypatch, tmp_path): + """No CTRL_BREAK_EVENT dependency here -- there is no pid to signal, so + this one runs everywhere, same as the pre-existing behaviour it replaces.""" + monkeypatch.setattr(sys, "platform", "win32") + monkeypatch.setenv("LOCALAPPDATA", str(tmp_path)) + run_calls = [] + monkeypatch.setattr(plat.subprocess, "run", + lambda argv, **kw: run_calls.append(argv)) + plat.autostart_end() + assert run_calls == [["taskkill", "/IM", "meshbay-node.exe", "/F"]] + + +def test_autostart_end_ignores_a_stale_pid_reused_by_another_process(monkeypatch, tmp_path): + """The recorded pid is alive but is not meshbay-node.exe -- Windows reused + it after the daemon exited. Must not send CTRL_BREAK_EVENT to whatever + that is; falls straight to taskkill (by image name, so harmless here).""" + monkeypatch.setattr(sys, "platform", "win32") + monkeypatch.setenv("LOCALAPPDATA", str(tmp_path)) + plat._pid_file().parent.mkdir(parents=True, exist_ok=True) + plat._pid_file().write_text("4242", encoding="utf-8") + + monkeypatch.setattr(plat, "_pid_is_meshbay_node", lambda pid: False) + kill_calls = [] + monkeypatch.setattr(plat.os, "kill", lambda pid, sig: kill_calls.append((pid, sig))) + run_calls = [] + monkeypatch.setattr(plat.subprocess, "run", + lambda argv, **kw: run_calls.append(argv)) + + plat.autostart_end() + + assert kill_calls == [] # never signalled the reused pid + assert run_calls == [["taskkill", "/IM", "meshbay-node.exe", "/F"]] + assert not plat._pid_file().exists() + + +def test_autostart_end_is_a_noop_off_windows(monkeypatch): + monkeypatch.setattr(sys, "platform", "linux") + run_calls = [] + monkeypatch.setattr(plat.subprocess, "run", + lambda argv, **kw: run_calls.append(argv)) + plat.autostart_end() + assert run_calls == [] + + +# ── Console close / logoff / shutdown handler ─────────────────────────────── + +def test_install_console_close_handler_is_a_noop_off_windows(monkeypatch): + monkeypatch.setattr(sys, "platform", "linux") + loop = Mock() + stop_event = Mock() + assert plat.install_console_close_handler(loop, stop_event) is None + + +@pytest.mark.skipif(sys.platform != "win32", + reason="ctypes.windll/wintypes exist only on win32") +def test_install_console_close_handler_registers_and_the_callback_sets_stop_event(): + import asyncio as _asyncio + + loop = _asyncio.new_event_loop() + try: + stop_event = _asyncio.Event() + shutdown_done = plat.install_console_close_handler(loop, stop_event) + assert shutdown_done is not None + # Drive the registered handler directly rather than actually closing a + # console window -- exercises the same code path SetConsoleCtrlHandler + # would invoke, without needing a live console to close. + handler = plat._CONSOLE_HANDLER_REFS[-1] + shutdown_done.set() # so the handler's bounded wait returns immediately + # ctypes marshals the WINFUNCTYPE's BOOL restype back as a plain int + # (1/0), not a Python bool, when called directly like this. + assert handler(plat.CTRL_CLOSE_EVENT) + loop.run_until_complete(_asyncio.sleep(0)) # let call_soon_threadsafe land + assert stop_event.is_set() + # An event this handler does not own (CTRL_C_EVENT) is left unhandled + # so Python's own console handler (or the default action) gets it. + assert not handler(0) + finally: + loop.close() + + +# ── Service mode ───────────────────────────────────────────────────────────── + +def test_service_install_removes_the_startup_launcher_first(win_startup, monkeypatch): + """The two mechanisms are mutually exclusive by design -- both installed + would start the daemon twice, once at boot and again at sign-in. This is + the CLI's own front door to that invariant, separate from (but agreeing + with) the Node page's startup-mode selector.""" + plat.autostart_install(exe=r"C:\x\meshbay-node.exe") + assert win_startup.exists() + + monkeypatch.setattr(plat, "_current_user", lambda: "DOMAIN\\user") + calls = [] + monkeypatch.setattr( + plat, "_schtasks", + lambda *args: calls.append(args) or Mock(returncode=0, stdout="", stderr="")) + + plat.service_install(exe=r"C:\x\meshbay-node.exe") + + assert not win_startup.exists() # removed as part of service_install + assert calls and calls[0][0] == "/create" + + +def test_service_install_tolerates_no_startup_launcher_present(win_startup, monkeypatch): + assert not win_startup.exists() + monkeypatch.setattr(plat, "_current_user", lambda: "DOMAIN\\user") + monkeypatch.setattr(plat, "_schtasks", + lambda *args: Mock(returncode=0, stdout="", stderr="")) + plat.service_install(exe=r"C:\x\meshbay-node.exe") # no error + assert not win_startup.exists() + + # ── Packaged defaults ──────────────────────────────────────────────────────── def test_frozen_build_finds_default_env_beside_the_executable(monkeypatch, tmp_path): -- cgit v1.2.3