diff --git a/CHANGELOG.md b/CHANGELOG.md index ddf0c27cd..65b04cfa5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ the frozen-backend fallback mirror it for their toolchains. - The OmniVoice guide now covers combining style attributes with a reference clip (consistent instruct stabilizes cloning; the reference wins conflicts), inline pronunciation control (pinyin / CMU phonemes), and corrects the claim that the default engine can't do voice design — it can, from attributes (#1565) ### Fixed +- Desktop startup, Retry, reset, uninstall, shutdown, and crash recovery now share one backend lifecycle owner; quitting interrupts first-run installers and gracefully drains then force-cleans the full backend process tree, so overlaps cannot duplicate or orphan it (#1635) — thanks @Xohaibxobi! - Large Stories and Audiobook projects now persist in IndexedDB instead of overflowing the `omnivoice.app` localStorage envelope, with quota-safe migration and orderly exit/reload flushing (#1636) — thanks @leodzai! - OmniVoice and its crash-isolated subprocess now route to AMD ROCm GPUs instead of warning and falling back to CPU (#1629) — thanks @j4r3kb! - Dictation now cancels pending startup work, capture resources, sockets, and timers when the capture widget closes, preventing late work against a destroyed webview (#1645) diff --git a/backend/core/contained_subprocess.py b/backend/core/contained_subprocess.py new file mode 100644 index 000000000..15754e3b4 --- /dev/null +++ b/backend/core/contained_subprocess.py @@ -0,0 +1,557 @@ +"""Nested subprocess ownership for desktop-managed backend operations. + +The desktop owns the backend with an OS process group/Job. Engine and +installer operations also need an independently terminable subtree: killing +only their direct child on a timeout leaves uv/git/model workers holding pipes +and mutating files. A small direct-child supervisor bridges both lifetimes. + +On POSIX the supervisor is the unreaped leader of a nested process group. A +control-pipe EOF (including kernel EOF when the backend dies) kills that group; +the parent also drains the group before reaping its stable leader. On Windows +the supervisor assigns the operation, while suspended, to a nested +kill-on-close Job. The outer desktop Job still contains both levels. + +Standalone/server launches use the same nested owner, preserving their +independently terminable subtree without relying on ``taskkill`` or discovery. +""" +from __future__ import annotations + +import os +import signal +import struct +import subprocess +import sys +import threading +import time +from pathlib import Path +from typing import Any, Optional + + +_RESULT = struct.Struct("!i") +_DESKTOP_MARKER = "OMNIVOICE_DESKTOP_CONTAINED" +_DRAIN_FD_ENV = "OMNIVOICE_DESKTOP_DRAIN_FD" + + +def backend_drain_fd(*, required: bool = False) -> Optional[int]: + """Validated Rust-owned drain writer inherited by the desktop backend.""" + if os.name != "posix" or os.environ.get(_DESKTOP_MARKER) != "1": + return None + try: + fd = int(os.environ[_DRAIN_FD_ENV]) + os.fstat(fd) + except (KeyError, ValueError, OSError) as exc: + if required: + raise RuntimeError( + "desktop backend is missing its live nested-operation drain descriptor" + ) from exc + return None + return fd + + +def secure_backend_drain_fd() -> None: + """Restore CLOEXEC after Rust's one intentional backend inheritance.""" + fd = backend_drain_fd(required=True) + if fd is not None: + os.set_inheritable(fd, False) + + +class OwnedPopen: + """Popen-compatible handle for a desktop-owned nested operation.""" + + def __init__( + self, + proc: subprocess.Popen, + control_fd: int, + result_fd: int, + ) -> None: + self._proc = proc + self._control_fd: Optional[int] = control_fd + self._result_fd: Optional[int] = result_fd + self._returncode: Optional[int] = None + self._lock = threading.RLock() + + # Popen callers use these directly (protocol pipes and log drains). + self.stdin = proc.stdin + self.stdout = proc.stdout + self.stderr = proc.stderr + + @property + def pid(self) -> int: + return self._proc.pid + + @property + def args(self) -> Any: + return self._proc.args + + @property + def returncode(self) -> Optional[int]: + return self._returncode + + def _close_control(self) -> None: + fd, self._control_fd = self._control_fd, None + if fd is not None: + try: + os.close(fd) + except OSError: + # Cleanup is idempotent; another teardown path already closed it. + pass + + def _read_result(self, fallback: int) -> int: + fd, self._result_fd = self._result_fd, None + if fd is None: + return fallback + try: + payload = b"" + while len(payload) < _RESULT.size: + chunk = os.read(fd, _RESULT.size - len(payload)) + if not chunk: + break + payload += chunk + return _RESULT.unpack(payload)[0] if len(payload) == _RESULT.size else fallback + except OSError: + return fallback + finally: + try: + os.close(fd) + except OSError: + # The descriptor may have been closed by cancellation cleanup. + pass + + def _posix_exited_unreaped(self) -> bool: + flags = os.WEXITED | os.WNOHANG | os.WNOWAIT + info = os.waitid(os.P_PID, self.pid, flags) + return info is not None and info.si_pid != 0 + + def _signal_owned_group(self, sig: int) -> None: + # The numeric group is safe only while its direct-child leader remains + # ours and unreaped. ECHILD therefore refuses rather than guessing. + try: + os.waitid(os.P_PID, self.pid, os.WEXITED | os.WNOHANG | os.WNOWAIT) + except ChildProcessError: + return + try: + os.killpg(self.pid, sig) + except ProcessLookupError: + # The owned group exited between the waitid probe and the signal. + pass + + def poll(self) -> Optional[int]: + with self._lock: + if self._returncode is not None: + return self._returncode + if os.name == "posix": + try: + if not self._posix_exited_unreaped(): + return None + except ChildProcessError: + # Never signal a potentially reused group after another + # owner reaped the stable leader. + return None + self._signal_owned_group(signal.SIGKILL) + wrapper_rc = self._proc.wait() + else: + wrapper_rc = self._proc.poll() + if wrapper_rc is None: + return None + self._close_control() + self._returncode = self._read_result(wrapper_rc) + return self._returncode + + def wait(self, timeout: Optional[float] = None) -> int: + deadline = None if timeout is None else time.monotonic() + timeout + while True: + rc = self.poll() + if rc is not None: + return rc + if deadline is not None and time.monotonic() >= deadline: + raise subprocess.TimeoutExpired(self.args, timeout) + time.sleep(0.01) + + def terminate(self) -> None: + with self._lock: + if self._returncode is not None: + return + self._close_control() + if os.name == "posix": + self._signal_owned_group(signal.SIGTERM) + else: + # Closing the control pipe asks the supervisor to terminate + # its nested Job. The stable wrapper handle is a fallback. + try: + self._proc.terminate() + except OSError: + # The wrapper exited after the return-code check. + pass + + def kill(self) -> None: + with self._lock: + if self._returncode is not None: + return + self._close_control() + if os.name == "posix": + self._signal_owned_group(signal.SIGKILL) + else: + try: + self._proc.kill() + except OSError: + # The wrapper exited after the return-code check. + pass + + def __getattr__(self, name: str) -> Any: + return getattr(self._proc, name) + + def __del__(self) -> None: + self._close_control() + fd, self._result_fd = self._result_fd, None + if fd is not None: + try: + os.close(fd) + except OSError: + # Finalization may race explicit wait or cancellation cleanup. + pass + + +def spawn_owned(argv: list[str], **kwargs: Any) -> "subprocess.Popen | OwnedPopen": + """Spawn an operation with a stable, independently terminable owner.""" + + drain_fd = backend_drain_fd(required=True) if os.name == "posix" else None + control_read, control_write = os.pipe() + result_read, result_write = os.pipe() + control_token = control_read + result_token = result_write + if os.name == "nt": + import msvcrt + + control_token = msvcrt.get_osfhandle(control_read) + result_token = msvcrt.get_osfhandle(result_write) + wrapper_argv = _supervisor_argv( + control_token, + result_token, + argv, + ) + wrapper_kwargs = dict(kwargs) + if os.name == "posix": + wrapper_kwargs["start_new_session"] = True + pass_fds = [control_read, result_write] + if drain_fd is not None: + pass_fds.append(drain_fd) + if wrapper_kwargs.get("env") is not None: + wrapper_env = dict(wrapper_kwargs["env"]) + wrapper_env[_DESKTOP_MARKER] = "1" + wrapper_env[_DRAIN_FD_ENV] = str(drain_fd) + wrapper_kwargs["env"] = wrapper_env + wrapper_kwargs["pass_fds"] = tuple(pass_fds) + else: + # Python's Windows fd inheritance requires inheritable CRT handles. + # All unrelated descriptors are non-inheritable by default (PEP 446). + os.set_handle_inheritable(control_token, True) + os.set_handle_inheritable(result_token, True) + wrapper_kwargs["close_fds"] = False + try: + proc = subprocess.Popen(wrapper_argv, **wrapper_kwargs) + except BaseException: + # The finally block exclusively owns the child-side endpoints. Closing + # them here as well risks closing a reused descriptor in another thread. + for fd in (control_write, result_read): + try: + os.close(fd) + except OSError: + # A partial spawn may already have closed a parent-side endpoint. + pass + raise + finally: + for fd in (control_read, result_write): + try: + os.close(fd) + except OSError: + # Popen may have consumed an inherited child-side endpoint. + pass + return OwnedPopen(proc, control_write, result_read) + + +def _supervisor_argv( + control_token: int, + result_token: int, + argv: list[str], +) -> list[str]: + prefix = [sys.executable] + if not getattr(sys, "frozen", False): + prefix.append(str(Path(__file__).resolve().parents[1] / "main.py")) + return [ + *prefix, + "--supervise", + str(control_token), + str(result_token), + "--", + *map(str, argv), + ] + + +def _write_result(fd: int, returncode: int) -> None: + try: + os.write(fd, _RESULT.pack(int(returncode))) + except OSError: + # The caller may have cancelled and closed its result reader. + pass + finally: + try: + os.close(fd) + except OSError: + # Writing or cancellation may already have closed the descriptor. + pass + + +def _operation_env() -> dict[str, str]: + env = os.environ.copy() + # The operation intentionally does not own the Rust drain writer. Avoid + # exposing a stale numeric token which nested code could mistake as valid. + env.pop(_DRAIN_FD_ENV, None) + env.pop(_DESKTOP_MARKER, None) + return env + + +def _supervise_posix(control_fd: int, result_fd: int, argv: list[str]) -> int: + def cancel_on_eof() -> None: + try: + while os.read(control_fd, 1): + pass + except OSError: + # Closing the control descriptor is itself a cancellation signal. + pass + os.killpg(os.getpgrp(), signal.SIGKILL) + + threading.Thread(target=cancel_on_eof, daemon=True).start() + try: + child = subprocess.Popen(argv, close_fds=True, env=_operation_env()) + rc = child.wait() + except OSError: + rc = 127 + _write_result(result_fd, rc) + # Drain children which outlived the operation before the stable group + # leader exits. SIGKILL intentionally includes this supervisor. + os.killpg(os.getpgrp(), signal.SIGKILL) + return rc # unreachable + + +def _windows_job() -> tuple[Any, Any, Any]: + import ctypes + import ctypes.wintypes as wintypes + + kernel32 = ctypes.WinDLL("kernel32", use_last_error=True) + kernel32.CloseHandle.argtypes = (wintypes.HANDLE,) + kernel32.CloseHandle.restype = wintypes.BOOL + kernel32.TerminateJobObject.argtypes = (wintypes.HANDLE, wintypes.UINT) + kernel32.TerminateJobObject.restype = wintypes.BOOL + kernel32.ReadFile.argtypes = ( + wintypes.HANDLE, + ctypes.c_void_p, + wintypes.DWORD, + ctypes.POINTER(wintypes.DWORD), + ctypes.c_void_p, + ) + kernel32.ReadFile.restype = wintypes.BOOL + kernel32.WriteFile.argtypes = ( + wintypes.HANDLE, + ctypes.c_void_p, + wintypes.DWORD, + ctypes.POINTER(wintypes.DWORD), + ctypes.c_void_p, + ) + kernel32.WriteFile.restype = wintypes.BOOL + create = kernel32.CreateJobObjectW + create.argtypes = (ctypes.c_void_p, wintypes.LPCWSTR) + create.restype = wintypes.HANDLE + job = create(None, None) + if not job: + raise OSError(ctypes.get_last_error(), "CreateJobObjectW") + + class BasicLimits(ctypes.Structure): + _fields_ = [ + ("PerProcessUserTimeLimit", ctypes.c_longlong), + ("PerJobUserTimeLimit", ctypes.c_longlong), + ("LimitFlags", wintypes.DWORD), + ("MinimumWorkingSetSize", ctypes.c_size_t), + ("MaximumWorkingSetSize", ctypes.c_size_t), + ("ActiveProcessLimit", wintypes.DWORD), + ("Affinity", ctypes.c_size_t), + ("PriorityClass", wintypes.DWORD), + ("SchedulingClass", wintypes.DWORD), + ] + + class IoCounters(ctypes.Structure): + _fields_ = [(name, ctypes.c_ulonglong) for name in ( + "ReadOperationCount", "WriteOperationCount", "OtherOperationCount", + "ReadTransferCount", "WriteTransferCount", "OtherTransferCount", + )] + + class ExtendedLimits(ctypes.Structure): + _fields_ = [ + ("BasicLimitInformation", BasicLimits), + ("IoInfo", IoCounters), + ("ProcessMemoryLimit", ctypes.c_size_t), + ("JobMemoryLimit", ctypes.c_size_t), + ("PeakProcessMemoryUsed", ctypes.c_size_t), + ("PeakJobMemoryUsed", ctypes.c_size_t), + ] + + info = ExtendedLimits() + info.BasicLimitInformation.LimitFlags = 0x00002000 # KILL_ON_JOB_CLOSE + set_info = kernel32.SetInformationJobObject + set_info.argtypes = (wintypes.HANDLE, ctypes.c_int, ctypes.c_void_p, wintypes.DWORD) + set_info.restype = wintypes.BOOL + if not set_info(job, 9, ctypes.byref(info), ctypes.sizeof(info)): + error = ctypes.get_last_error() + kernel32.CloseHandle(job) + raise OSError(error, "SetInformationJobObject") + return job, kernel32, wintypes + + +def _resume_windows_process(kernel32: Any, wintypes: Any, pid: int) -> None: + import ctypes + + class ThreadEntry(ctypes.Structure): + _fields_ = [ + ("dwSize", wintypes.DWORD), + ("cntUsage", wintypes.DWORD), + ("th32ThreadID", wintypes.DWORD), + ("th32OwnerProcessID", wintypes.DWORD), + ("tpBasePri", wintypes.LONG), + ("tpDeltaPri", wintypes.LONG), + ("dwFlags", wintypes.DWORD), + ] + + kernel32.CreateToolhelp32Snapshot.argtypes = (wintypes.DWORD, wintypes.DWORD) + kernel32.CreateToolhelp32Snapshot.restype = wintypes.HANDLE + kernel32.Thread32First.argtypes = (wintypes.HANDLE, ctypes.POINTER(ThreadEntry)) + kernel32.Thread32First.restype = wintypes.BOOL + kernel32.Thread32Next.argtypes = (wintypes.HANDLE, ctypes.POINTER(ThreadEntry)) + kernel32.Thread32Next.restype = wintypes.BOOL + kernel32.OpenThread.argtypes = (wintypes.DWORD, wintypes.BOOL, wintypes.DWORD) + kernel32.OpenThread.restype = wintypes.HANDLE + kernel32.ResumeThread.argtypes = (wintypes.HANDLE,) + kernel32.ResumeThread.restype = wintypes.DWORD + + snapshot = kernel32.CreateToolhelp32Snapshot(0x00000004, 0) + invalid = ctypes.c_void_p(-1).value + if snapshot == invalid: + raise OSError(ctypes.get_last_error(), "CreateToolhelp32Snapshot") + try: + entry = ThreadEntry(dwSize=ctypes.sizeof(ThreadEntry)) + found = kernel32.Thread32First(snapshot, ctypes.byref(entry)) + while found: + if entry.th32OwnerProcessID == pid: + thread = kernel32.OpenThread(0x0002, False, entry.th32ThreadID) + if not thread: + raise OSError(ctypes.get_last_error(), "OpenThread") + try: + if kernel32.ResumeThread(thread) == 0xFFFFFFFF: + raise OSError(ctypes.get_last_error(), "ResumeThread") + return + finally: + kernel32.CloseHandle(thread) + found = kernel32.Thread32Next(snapshot, ctypes.byref(entry)) + finally: + kernel32.CloseHandle(snapshot) + raise OSError("suspended operation thread was not found") + + +def _supervise_windows(control_fd: int, result_fd: int, argv: list[str]) -> int: + import ctypes + + job, kernel32, wintypes = _windows_job() + cancelled = threading.Event() + job_lock = threading.Lock() + job_open = True + + def terminate_job() -> None: + with job_lock: + if job_open: + kernel32.TerminateJobObject(job, 1) + + def cancel_on_eof() -> None: + byte = ctypes.create_string_buffer(1) + count = wintypes.DWORD() + while kernel32.ReadFile( + wintypes.HANDLE(control_fd), byte, 1, ctypes.byref(count), None + ) and count.value: + pass + kernel32.CloseHandle(wintypes.HANDLE(control_fd)) + cancelled.set() + terminate_job() + + threading.Thread(target=cancel_on_eof, daemon=True).start() + child: Optional[subprocess.Popen] = None + rc = 127 + try: + child = subprocess.Popen( + argv, + close_fds=True, + env=_operation_env(), + creationflags=0x08000000 | 0x00000004, # NO_WINDOW | SUSPENDED + ) + assign = kernel32.AssignProcessToJobObject + assign.argtypes = (wintypes.HANDLE, wintypes.HANDLE) + assign.restype = wintypes.BOOL + if not assign(job, wintypes.HANDLE(child._handle)): + raise OSError(ctypes.get_last_error(), "AssignProcessToJobObject") + if cancelled.is_set(): + terminate_job() + else: + _resume_windows_process(kernel32, wintypes, child.pid) + rc = child.wait() + # A successful direct child may leave helpers behind; terminate the + # nested stable Job before reporting completion. + terminate_job() + except OSError: + terminate_job() + if child is not None: + try: + # Assignment itself may have failed, leaving this suspended + # process outside the nested Job. Terminate it through its + # stable process handle before waiting; never strand an + # unassigned operation or rely on the outer desktop Job. + child.kill() + except OSError: + # The suspended child may have exited during Job teardown. + pass + try: + child.wait(timeout=5) + except (OSError, subprocess.TimeoutExpired): + # The outer desktop Job remains the terminal containment fallback. + pass + finally: + payload = _RESULT.pack(int(rc)) + payload_buffer = ctypes.create_string_buffer(payload) + written = wintypes.DWORD() + kernel32.WriteFile( + wintypes.HANDLE(result_fd), + payload_buffer, + len(payload), + ctypes.byref(written), + None, + ) + kernel32.CloseHandle(wintypes.HANDLE(result_fd)) + with job_lock: + job_open = False + kernel32.CloseHandle(job) + return rc + + +def supervisor_main(args: list[str]) -> int: + if len(args) < 5 or args[0] != "--supervise" or args[3] != "--": + return 2 + control_fd = int(args[1]) + result_fd = int(args[2]) + argv = args[4:] + secure_backend_drain_fd() + if os.name == "posix": + return _supervise_posix(control_fd, result_fd, argv) + return _supervise_windows(control_fd, result_fd, argv) + + +def _main() -> int: + return supervisor_main(sys.argv[1:]) + + +if __name__ == "__main__": + raise SystemExit(_main()) diff --git a/backend/main.py b/backend/main.py index 3e2dce021..0fddde026 100644 --- a/backend/main.py +++ b/backend/main.py @@ -1,4 +1,3 @@ -import math import os import sys @@ -10,6 +9,24 @@ if _backend_dir not in sys.path: sys.path.insert(0, _backend_dir) +# PyInstaller re-executes this entry module when the frozen backend binary is +# launched. Nested operation supervisors therefore dispatch here, before math, +# logging, FastAPI, torch, or any application initialization. Source launches +# use this same entry contract so frozen/source behavior cannot drift. +if __name__ == "__main__" and len(sys.argv) > 1 and sys.argv[1] == "--supervise": + from core.contained_subprocess import supervisor_main + + raise SystemExit(supervisor_main(sys.argv[1:])) + +# Rust clears CLOEXEC only for the backend exec. Re-arm PEP 446 immediately: +# nested supervisors receive this descriptor solely through explicit pass_fds, +# so a third-party close_fds=False child cannot hold the desktop drain barrier. +from core.contained_subprocess import secure_backend_drain_fd # noqa: E402 + +secure_backend_drain_fd() + +import math # noqa: E402 + # Windows: run every child process (ffmpeg, engine sidecars, yt-dlp, demucs, …) # WITHOUT popping a console window. The backend itself is spawned console-less by # the Tauri shell, so on Windows each console subprocess it launches would diff --git a/backend/services/sidecar_install.py b/backend/services/sidecar_install.py index 96d8e4cf4..57adfdd03 100644 --- a/backend/services/sidecar_install.py +++ b/backend/services/sidecar_install.py @@ -60,6 +60,7 @@ from typing import Callable, Optional from core.config import DATA_DIR +from core.contained_subprocess import OwnedPopen, spawn_owned logger = logging.getLogger("omnivoice.sidecar_install") @@ -998,6 +999,11 @@ def _step_persist(spec: SidecarSpec, job: dict) -> None: # ── Subprocess runner with live log capture ──────────────────────────────── +def _install_containment_kwargs() -> dict: + """Nested process-group/Job ownership is supplied by ``spawn_owned``.""" + return {} + + def _run_logged(job: dict, argv: list[str], *, timeout: float, env: "dict[str, str] | None" = None) -> int: """Run *argv*, streaming combined stdout+stderr lines into the job log. @@ -1012,13 +1018,11 @@ def _run_logged(job: dict, argv: list[str], *, timeout: float, killed child — a blocking ``for line in proc.stdout`` on this thread would hang past the timeout waiting for pipe EOF. """ - popen_kwargs: dict = {} - if os.name == "posix": - # New session → we can kill the whole process group on timeout - # instead of only the direct child. - popen_kwargs["start_new_session"] = True + # ``spawn_owned`` creates the local timeout group/Job before the operation + # starts and links it to backend death through its control pipe. + popen_kwargs = _install_containment_kwargs() try: - proc = subprocess.Popen( + proc = spawn_owned( argv, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, @@ -1053,29 +1057,18 @@ def _drain() -> None: def _kill_tree(proc: "subprocess.Popen") -> None: - """Kill the child and its whole process tree, on every platform. - - POSIX: the child was started in its own session, so SIGKILL the group. - Windows: ``proc.kill()`` only terminates the direct child — a git/uv - helper it spawned would keep running (and writing into the checkout) - past our timeout — so use ``taskkill /T`` to fell the tree. - """ - if os.name == "posix": - import signal - try: - os.killpg(proc.pid, signal.SIGKILL) - return - except (ProcessLookupError, PermissionError, OSError): - pass # group already gone / not ours — fall through to plain kill - else: # Windows + """Kill an operation through its stable nested group/Job owner.""" + if isinstance(proc, OwnedPopen): + # The retained supervisor/process-group or nested Job is the stable + # per-operation owner. Do not fall back to a direct PID kill. + proc.kill() try: - subprocess.run( - ["taskkill", "/F", "/T", "/PID", str(proc.pid)], - capture_output=True, timeout=15, - ) - return - except (OSError, subprocess.SubprocessError): - pass # taskkill unavailable/failed — fall through to plain kill + proc.wait(timeout=5) + except subprocess.TimeoutExpired: + pass + return + # A test double or a legacy caller without the nested owner can only be + # stopped through its stable direct-process handle. try: proc.kill() except OSError: diff --git a/backend/services/subprocess_backend.py b/backend/services/subprocess_backend.py index bc2786709..5fde72178 100644 --- a/backend/services/subprocess_backend.py +++ b/backend/services/subprocess_backend.py @@ -32,9 +32,9 @@ def sidecar_script(cls) -> Path: ... # path to backend/engines//main.py AUTH-05 installed (``HFTokenRedactor``) on the root logger. T-02-04 — compromised sidecar emitting unexpected ops: parent allowlist ``PARENT_INBOUND_OPS`` rejects everything else. - T-02-05 — Tauri group-kill scope: ``start_new_session=True`` on Unix - and ``CREATE_NEW_PROCESS_GROUP`` on Windows isolate the - sidecar's process group. + T-02-05 — nested containment: a retained supervisor process group/Job owns + each engine operation and is linked to backend death by a control + pipe, while still permitting independent timeout teardown. """ from __future__ import annotations @@ -56,6 +56,7 @@ def sidecar_script(cls) -> Path: ... # path to backend/engines//main.py import numpy as np import torch +from core.contained_subprocess import spawn_owned from services.tts_backend import TTSBackend logger = logging.getLogger("omnivoice.subprocess_backend") @@ -470,13 +471,6 @@ def _spawn(self) -> None: "env": env, "bufsize": 0, # unbuffered binary pipes } - # Process-group isolation so the Tauri lib.rs group-kill in shutdown - # doesn't escape into other children. See T-02-05. - if sys.platform == "win32": - kwargs["creationflags"] = subprocess.CREATE_NEW_PROCESS_GROUP - else: - kwargs["start_new_session"] = True - # `venv_python()` resolves the engine's interpreter, and on a cold # first run that is not cheap: it spawns each candidate to import the # engine (bounded, but tens of seconds on a slow disk), and if none is @@ -509,7 +503,7 @@ def _spawn(self) -> None: self.id, Path(python_path).name, Path(script_path).name, ) try: - self._proc = subprocess.Popen([python_path, script_path], **kwargs) + self._proc = spawn_owned([python_path, script_path], **kwargs) except OSError as exc: raise InvalidBinaryError( python_path, diff --git a/backend/tests/test_contained_subprocess.py b/backend/tests/test_contained_subprocess.py new file mode 100644 index 000000000..2b9f046a0 --- /dev/null +++ b/backend/tests/test_contained_subprocess.py @@ -0,0 +1,259 @@ +"""Stable nested operation ownership (model-free, cross-platform seams).""" +import ctypes +import builtins +import os +import runpy +import subprocess +import sys +import threading +import time +import types +from ctypes import wintypes +from pathlib import Path + +import pytest + +from core import contained_subprocess as owned + + +class _Call: + def __init__(self, fn): + self.fn = fn + + def __call__(self, *args): + return self.fn(*args) + + +def test_supervisor_argv_uses_entry_module_for_source_and_frozen_binary(monkeypatch): + monkeypatch.delattr(owned.sys, "frozen", raising=False) + source = owned._supervisor_argv(3, 4, ["operation"]) + assert source[:2] == [sys.executable, str(Path(owned.__file__).parents[1] / "main.py")] + assert source[2:] == ["--supervise", "3", "4", "--", "operation"] + + monkeypatch.setattr(owned.sys, "frozen", True, raising=False) + frozen = owned._supervisor_argv(3, 4, ["operation"]) + assert frozen == [sys.executable, "--supervise", "3", "4", "--", "operation"] + + +def test_source_main_dispatches_supervisor_before_heavy_imports(monkeypatch): + calls = [] + fake = types.ModuleType("core.contained_subprocess") + fake.supervisor_main = lambda args: calls.append(args) or 23 + monkeypatch.setitem(sys.modules, "core.contained_subprocess", fake) + main_path = Path(owned.__file__).parents[1] / "main.py" + monkeypatch.setattr( + sys, + "argv", + [str(main_path), "--supervise", "3", "4", "--", "operation"], + ) + original_import = builtins.__import__ + + def guard_heavy_import(name, *args, **kwargs): + if name == "math": + raise AssertionError("supervisor dispatch reached application imports") + return original_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", guard_heavy_import) + with pytest.raises(SystemExit, match="23"): + runpy.run_path(str(main_path), run_name="__main__") + assert calls == [["--supervise", "3", "4", "--", "operation"]] + + +@pytest.mark.skipif(os.name != "posix", reason="Unix drain pipe contract") +def test_drain_fd_is_explicitly_inherited_by_wrapper_but_not_operation(monkeypatch): + drain_read, drain_write = os.pipe() + monkeypatch.setenv("OMNIVOICE_DESKTOP_CONTAINED", "1") + monkeypatch.setenv("OMNIVOICE_DESKTOP_DRAIN_FD", str(drain_write)) + owned.secure_backend_drain_fd() + assert not os.get_inheritable(drain_write) + implicit_probe = subprocess.check_output( + [ + sys.executable, + "-c", + "import os; " + "fd=int(os.environ['OMNIVOICE_DESKTOP_DRAIN_FD']); " + "\ntry: os.fstat(fd); print('leaked')" + "\nexcept OSError: print('closed')", + ], + close_fds=False, + text=True, + ) + assert implicit_probe.strip() == "closed" + script = ( + "import os,time; token=os.environ.get('OMNIVOICE_DESKTOP_DRAIN_FD'); " + "marker=os.environ.get('OMNIVOICE_DESKTOP_CONTAINED'); " + "\nif token is None and marker is None: state='stripped'" + "\nelse:" + "\n try: os.fstat(int(token)); state='leaked'" + "\n except OSError: state='closed'" + "\nprint(state, flush=True); time.sleep(60)" + ) + proc = owned.spawn_owned( + [sys.executable, "-c", script], + stdout=subprocess.PIPE, + text=True, + ) + try: + assert proc.stdout.readline().strip() == "stripped" + os.close(drain_write) + drain_write = -1 + os.set_blocking(drain_read, False) + with pytest.raises(BlockingIOError): + os.read(drain_read, 1) # wrapper still holds the only writer + proc.kill() + proc.wait(timeout=5) + deadline = time.monotonic() + 2 + while time.monotonic() < deadline: + try: + if os.read(drain_read, 1) == b"": + break + except BlockingIOError: + time.sleep(0.01) + else: + pytest.fail("wrapper exit did not close the desktop drain writer") + finally: + if drain_write >= 0: + os.close(drain_write) + os.close(drain_read) + if proc.poll() is None: + proc.kill() + proc.wait(timeout=5) + + +def test_invalid_or_missing_desktop_drain_fd_fails_safe(monkeypatch): + monkeypatch.setenv("OMNIVOICE_DESKTOP_CONTAINED", "1") + monkeypatch.setenv("OMNIVOICE_DESKTOP_DRAIN_FD", "not-an-fd") + with pytest.raises(RuntimeError, match="missing its live.*drain descriptor"): + owned.spawn_owned([sys.executable, "-c", "print('unsafe')"]) + + monkeypatch.delenv("OMNIVOICE_DESKTOP_DRAIN_FD") + with pytest.raises(RuntimeError, match="missing its live.*drain descriptor"): + owned.secure_backend_drain_fd() + + monkeypatch.delenv("OMNIVOICE_DESKTOP_CONTAINED") + assert owned.backend_drain_fd(required=True) is None + proc = owned.spawn_owned( + [sys.executable, "-c", "print('standalone')"], + stdout=subprocess.PIPE, + text=True, + ) + assert proc.stdout.readline().strip() == "standalone" + assert proc.wait(timeout=5) == 0 + + +def test_windows_operation_is_in_kill_on_close_job_before_resume(monkeypatch): + """The child gets no instruction before stable nested Job assignment.""" + events = [] + job_closed = threading.Event() + job = 99 + + def close_handle(handle): + value = getattr(handle, "value", handle) + events.append(("close", value)) + if value == job: + job_closed.set() + return True + + kernel = type("Kernel", (), {})() + kernel.AssignProcessToJobObject = _Call( + lambda assigned_job, process: events.append(("assign", assigned_job, process)) or True + ) + kernel.TerminateJobObject = _Call( + lambda assigned_job, code: events.append(("terminate", assigned_job, code)) or True + ) + kernel.WriteFile = _Call( + lambda handle, payload, size, written, overlap: events.append(("write", size)) or True + ) + kernel.CloseHandle = _Call(close_handle) + + def read_control(*_args): + job_closed.wait(2) + return False + + kernel.ReadFile = _Call(read_control) + monkeypatch.setattr(owned, "_windows_job", lambda: (job, kernel, wintypes)) + monkeypatch.setattr( + owned, + "_resume_windows_process", + lambda _kernel, _types, pid: events.append(("resume", pid)), + ) + + class Child: + _handle = 77 + pid = 123 + + def wait(self, timeout=None): + events.append(("wait", timeout)) + return 0 + + monkeypatch.setattr( + owned.subprocess, + "Popen", + lambda *args, **kwargs: events.append(("spawn", kwargs["creationflags"])) or Child(), + ) + + assert owned._supervise_windows(11, 12, ["operation.exe"]) == 0 + assert job_closed.wait(1) + + names = [event[0] for event in events] + assert names.index("assign") < names.index("resume") < names.index("wait") + assert names.index("wait") < names.index("terminate") < names.index("write") + + +def test_windows_assignment_failure_kills_suspended_unowned_child(monkeypatch): + """A child outside the nested Job must be killed through its stable handle.""" + events = [] + job_closed = threading.Event() + job = 99 + + def close_handle(handle): + value = getattr(handle, "value", handle) + events.append(("close", value)) + if value == job: + job_closed.set() + return True + + kernel = type("Kernel", (), {})() + kernel.AssignProcessToJobObject = _Call( + lambda assigned_job, process: events.append(("assign", assigned_job, process)) + or False + ) + kernel.TerminateJobObject = _Call( + lambda assigned_job, code: events.append(("terminate", assigned_job, code)) or True + ) + kernel.WriteFile = _Call( + lambda handle, payload, size, written, overlap: events.append(("write", size)) or True + ) + kernel.CloseHandle = _Call(close_handle) + + def read_control(*_args): + job_closed.wait(2) + return False + + kernel.ReadFile = _Call(read_control) + monkeypatch.setattr(owned, "_windows_job", lambda: (job, kernel, wintypes)) + monkeypatch.setattr(ctypes, "get_last_error", lambda: 5, raising=False) + + class Child: + _handle = 77 + pid = 123 + + def kill(self): + events.append(("kill",)) + + def wait(self, timeout=None): + events.append(("wait", timeout)) + return 1 + + monkeypatch.setattr( + owned.subprocess, + "Popen", + lambda *args, **kwargs: events.append(("spawn", kwargs["creationflags"])) or Child(), + ) + + assert owned._supervise_windows(11, 12, ["operation.exe"]) == 127 + assert job_closed.wait(1) + + names = [event[0] for event in events] + assert names.index("assign") < names.index("terminate") < names.index("kill") + assert names.index("kill") < names.index("wait") < names.index("write") diff --git a/backend/tests/test_omnivoice_subprocess.py b/backend/tests/test_omnivoice_subprocess.py index 8c3c6f1ac..955be2673 100644 --- a/backend/tests/test_omnivoice_subprocess.py +++ b/backend/tests/test_omnivoice_subprocess.py @@ -17,10 +17,19 @@ import math import array import base64 +import io +import os +import subprocess +import sys +import time +from pathlib import Path import pytest -from services.subprocess_backend import SubprocessBackend, RECV_TIMEOUT_S +from services.subprocess_backend import ( + RECV_TIMEOUT_S, + SubprocessBackend, +) from services.tts_backend import get_backend_class from engines.omnivoice_subprocess import OmniVoiceSubprocessBackend @@ -28,7 +37,7 @@ # ── stub sidecar (model-free) ────────────────────────────────────────────── STUB_SIDECAR = r''' -import sys, json, struct, time, math, array, base64 +import sys, os, json, struct, time, math, array, base64, subprocess def _send(o): b = json.dumps(o, separators=(",", ":")).encode() @@ -63,6 +72,15 @@ def _recv(): if t == "HANG": while True: # wedge forever; the parent must hard-kill us time.sleep(1) + if t == "HANG_CHILD": + subprocess.Popen([ + sys.executable, + "-c", + "import os,time; time.sleep(1); " + "open(os.environ['OMNIVOICE_TIMEOUT_MARKER'], 'w').write('bad')", + ]) + while True: + time.sleep(1) # Emit progress frames before the audio when asked, to exercise the # parent's progress-consuming recv loop (the cold-load fix). if t.startswith("PROG:"): @@ -136,6 +154,40 @@ def test_base_default_recv_timeout_is_60s(): assert _PlainBackend().recv_timeout_s == 60.0 +def test_sidecar_spawn_delegates_all_containment_to_nested_owner(monkeypatch, tmp_path): + from services import subprocess_backend as backend_module + + captured = {} + + class StubProcess: + stderr = io.BytesIO() + + @staticmethod + def poll(): + return None + + def fake_spawn(argv, **kwargs): + captured.update(kwargs) + return StubProcess() + + monkeypatch.setattr(_PlainBackend, "venv_python", classmethod(lambda cls: Path(sys.executable))) + monkeypatch.setattr( + _PlainBackend, + "sidecar_script", + classmethod(lambda cls: tmp_path / "stub.py"), + ) + monkeypatch.setattr(backend_module, "spawn_owned", fake_spawn) + monkeypatch.setattr(backend_module, "_ensure_reaper_running", lambda: None) + backend = _PlainBackend() + monkeypatch.setattr(backend, "_recv_with_timeout", lambda _timeout: {"op": "ready"}) + + try: + backend._spawn() + assert not ({"start_new_session", "creationflags", "preexec_fn"} & captured.keys()) + finally: + backend._proc = None + + def test_omnivoice_subprocess_recv_timeout_overrides_default(): b = OmniVoiceSubprocessBackend() assert b.recv_timeout_s == 300.0 # aligns with the generate budget @@ -205,6 +257,33 @@ def test_wedged_sidecar_is_hard_killed_and_recovers(stub_sidecar, monkeypatch): b.shutdown() +def test_desktop_timeout_kills_engine_subtree_before_late_mutation( + stub_sidecar, monkeypatch, tmp_path +): + marker = tmp_path / "late-engine-mutation" + monkeypatch.setenv("OMNIVOICE_DESKTOP_CONTAINED", "1") + drain_read, drain_write = os.pipe() + monkeypatch.setenv("OMNIVOICE_DESKTOP_DRAIN_FD", str(drain_write)) + monkeypatch.setenv("OMNIVOICE_TIMEOUT_MARKER", str(marker)) + _use_stub(monkeypatch, stub_sidecar) + monkeypatch.setattr( + OmniVoiceSubprocessBackend, + "recv_timeout_s", + property(lambda self: 0.3), + ) + b = OmniVoiceSubprocessBackend() + try: + with pytest.raises(RuntimeError): + b.generate("HANG_CHILD") + time.sleep(1.2) + assert not marker.exists() + assert b.generate("ok").shape[1] == 24000 + finally: + b.shutdown() + os.close(drain_write) + os.close(drain_read) + + def test_generate_does_not_deadlock_when_called_on_gpu_pool_worker(stub_sidecar, monkeypatch): # Regression: /v1/audio/speech and /generate dispatch backend.generate() via # run_on_gpu_pool_guarded, i.e. ON a gpu-pool worker. generate() must NOT diff --git a/docs/install/troubleshooting.md b/docs/install/troubleshooting.md index 46a72e0bd..b05051969 100644 --- a/docs/install/troubleshooting.md +++ b/docs/install/troubleshooting.md @@ -632,6 +632,20 @@ the error persistently on a current build, that's section **14** (a wedged GPU job), section **14d** (the backend never started), or the crash notice above — not this window. +Desktop startup, **Retry**, storage reset, setup re-entry, in-app uninstall, +app shutdown, and automatic crash recovery also share one backend lifecycle +owner. Overlapping start attempts wait and attach to the healthy process, +while reset/setup/uninstall keep exclusive ownership through teardown and +disk changes, and shutdown joins any in-progress launch before stopping it. +They no longer launch a second backend that fails on port 3900, leave a +misleading crash notice, delete an environment from under a starting child, +or orphan one on exit. Quitting also interrupts a first-run `uv` install +instead of waiting for a long download to finish. Unix builds give backend +lifespan cleanup a bounded SIGTERM grace period; Windows' hidden backend has no +console, so its initial stop is best-effort and may proceed directly to the +bounded forced tree cleanup. Surviving subprocess engines cannot retain ports +or files (#1635). + ## 14c. "Can't reach the backend" in a browser — `bun run dev`, Docker, or LAN share **Symptom:** you're using VoiceStudio **outside the desktop app** — the dev diff --git a/frontend/src-tauri/Cargo.toml b/frontend/src-tauri/Cargo.toml index a33217463..2dacf9c27 100644 --- a/frontend/src-tauri/Cargo.toml +++ b/frontend/src-tauri/Cargo.toml @@ -90,7 +90,7 @@ windows-core = "0.61" # `HWND` type — no second copy of the crate enters the dependency graph. # Win32_System_Registry: check_microphone reads the CapabilityAccessManager # ConsentStore mic toggle (RegGetValueW) for the permissions UX. -windows = { version = "0.61", features = ["Win32_Foundation", "Win32_UI_WindowsAndMessaging", "Win32_System_Registry", "Win32_System_Threading"] } +windows = { version = "0.61", features = ["Win32_Foundation", "Win32_UI_WindowsAndMessaging", "Win32_System_Registry", "Win32_System_Threading", "Win32_System_JobObjects", "Win32_System_Diagnostics_ToolHelp", "Win32_Security"] } [target.'cfg(unix)'.dependencies] libc = "0.2" diff --git a/frontend/src-tauri/src/backend.rs b/frontend/src-tauri/src/backend.rs index b66c1d87d..f2b07b983 100644 --- a/frontend/src-tauri/src/backend.rs +++ b/frontend/src-tauri/src/backend.rs @@ -5,7 +5,7 @@ use std::io::BufRead; use std::io::BufReader; use std::net::{TcpStream, ToSocketAddrs}; use std::path::{Path, PathBuf}; -use std::process::{Child, Command, Stdio}; +use std::process::{Command, Stdio}; use std::sync::{Arc, Mutex}; use std::time::Duration; @@ -199,18 +199,17 @@ fn raw_http_get(url: &str, timeout: Duration) -> Result { /// Keep in sync with `_EXIT_PORT_IN_USE` there. pub const EXIT_PORT_IN_USE: i32 = 78; -/// Kill whoever holds `port`, then confirm it actually came free. +/// Confirm whether `port` is free. An unowned listener is never killed by +/// numeric PID: even a successful HTTP identity probe cannot make a reusable +/// PID/process-group identifier safe to signal. /// /// #1223: every caller used to kill-then-sleep-then-spawn unconditionally, so -/// a holder we cannot kill — a different user's process, a `taskkill` blocked -/// by policy, a socket sitting in TIME_WAIT that the Windows `netstat` -/// LISTENING filter can't even see — was indistinguishable from success. The -/// backend then died on the bind with a raw errno and the user got "Backend -/// died (exit code 1)". +/// a holder we do not own was indistinguishable from success. The backend then +/// died on the bind with a raw errno and the user got "Backend died (exit code +/// 1)". /// -/// Returns true when the port is free afterwards. Polls rather than sleeping a -/// flat interval: the common case (our own orphan) frees in well under 500ms, -/// and the uncommon case deserves longer than one guess. +/// Returns true when the port is free afterwards. Polling accommodates the +/// short close handoff after a contained backend has just been drained. pub fn free_port_or_report(port: u16) -> bool { kill_orphan_on_port(port); for _ in 0..20 { @@ -220,69 +219,23 @@ pub fn free_port_or_report(port: u16) -> bool { std::thread::sleep(Duration::from_millis(100)); } log::error!( - "Port {} is still held after attempting to kill its owner — the \ - backend cannot bind it. Another application (or a process owned by a \ - different user) is using the port.", + "Port {} is still held by an unowned listener — the backend cannot \ + bind it. Quit the other VoiceStudio instance or application and try \ + again.", port ); false } -/// Kill whatever process owns the port. -#[cfg(unix)] +/// An HTTP response can justify attaching to a healthy same-version backend, +/// but never grants process ownership. Deliberately refuse orphan cleanup: +/// signalling a PID discovered through lsof/netstat has an unavoidable reuse +/// race, and a matching foreign service must never be terminated. pub fn kill_orphan_on_port(port: u16) { - if let Ok(out) = Command::new("lsof") - .args(["-ti", &format!(":{}", port)]) - .output() - { - if out.status.success() { - let pids = String::from_utf8_lossy(&out.stdout); - for pid in pids.split_whitespace() { - if let Ok(pid_n) = pid.parse::() { - log::warn!("Killing orphan process {} on port {}", pid_n, port); - unsafe { - libc::kill(pid_n, libc::SIGKILL); - } - } - } - } - } -} - -#[cfg(not(unix))] -pub fn kill_orphan_on_port(port: u16) { - // `netstat -ano` lists listening sockets with their owning PID. - // Parse the output to find the process listening on exactly `port`. - // no_window: this orphan-kill probe runs on every launch; without it a - // netstat console window flashes each time the app starts. - let out = match crate::tools::no_window(Command::new("netstat").args(["-ano", "-p", "TCP"])).output() { - Ok(o) => o, - Err(_) => return, - }; - let stdout = String::from_utf8_lossy(&out.stdout); - // Match the local address ending in ":PORT" exactly to avoid false - // positives (e.g. :3900 must not match port 39000). - let port_suffix = format!(":{}", port); - for line in stdout.lines() { - if !line.to_uppercase().contains("LISTENING") { - continue; - } - // Local address is the second whitespace-delimited field. - // Format: " TCP 0.0.0.0:3900 0.0.0.0:0 LISTENING 1234" - let local_addr = line.split_whitespace().nth(1).unwrap_or(""); - if !local_addr.ends_with(&port_suffix) { - continue; - } - let parts: Vec<&str> = line.split_whitespace().collect(); - if let Some(pid_str) = parts.last() { - if let Ok(pid) = pid_str.parse::() { - log::warn!("Killing orphan process {} on port {} (Windows)", pid, port); - let _ = crate::tools::no_window( - Command::new("taskkill").args(["/PID", &pid.to_string(), "/F"]), - ) - .output(); - } - } + if port_in_use(port) { + log::warn!( + "Refusing to signal the unowned listener on port {port}; only desktop-contained backends are terminable" + ); } } @@ -550,7 +503,10 @@ fn backend_cmd_override() -> Option> { parse_backend_cmd_override(&std::env::var("OMNIVOICE_BACKEND_CMD").ok()?) } -pub fn spawn_backend(app: &tauri::AppHandle, progress: Option<&Arc>>) -> Option { +pub(crate) fn spawn_backend( + app: &tauri::AppHandle, + progress: Option<&Arc>>, +) -> Option { let log_path = backend_log_path(); let err_path = log_path.with_file_name("backend_err.log"); log::info!( @@ -601,7 +557,12 @@ pub fn spawn_backend(app: &tauri::AppHandle, progress: Opt ); } - let mut env: Vec<(String, String)> = vec![("PYTHONUNBUFFERED".into(), "1".into())]; + let mut env: Vec<(String, String)> = vec![ + ("PYTHONUNBUFFERED".into(), "1".into()), + // Backend-managed engines/installers must inherit the desktop-owned + // process group/Job rather than escaping into a new session. + ("OMNIVOICE_DESKTOP_CONTAINED".into(), "1".into()), + ]; // Pin the child's OMNIVOICE_PORT to the value Rust resolved so Python's // network_share.backend_port() always agrees with the uvicorn --port we // pass below — otherwise a user-set OMNIVOICE_PORT would change the @@ -673,18 +634,6 @@ pub fn spawn_backend(app: &tauri::AppHandle, progress: Opt for (k, v) in &env { cmd.env(k, v); } - #[cfg(windows)] - { - use std::os::windows::process::CommandExt; - // CREATE_NO_WINDOW (0x08000000) | CREATE_NEW_PROCESS_GROUP (0x00000200). - // The backend used to inherit the app's console context, so OS console - // CLOSE/LOGOFF events could reach it and MKL's Fortran runtime aborted - // the process (`forrtl: error (200)`, exit 2 / 0xC000013A — #1153 - // class). No console + own process group = no console events, ever. - // stdout/stderr are piped above, so nothing is lost. Same flag the - // nvidia-smi probe already uses (setup.rs). - cmd.creation_flags(0x0800_0000 | 0x0000_0200); - } match cmd_override { Some(ref argv) => { cmd.args(&argv[1..]); @@ -703,16 +652,13 @@ pub fn spawn_backend(app: &tauri::AppHandle, progress: Opt ]); } } - let mut child = match cmd - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - { + cmd.stdout(Stdio::piped()).stderr(Stdio::piped()); + let mut contained = match crate::tools::spawn_process_tree(&mut cmd) { Ok(c) => { log::info!( "Backend started via venv python {} (pid {})", python.display(), - c.id() + c.child.id() ); c } @@ -736,7 +682,7 @@ pub fn spawn_backend(app: &tauri::AppHandle, progress: Opt } }; - if let Some(stdout_pipe) = child.stdout.take() { + if let Some(stdout_pipe) = contained.child.stdout.take() { let app_clone = app.clone(); let mut out_file = stdout_file; std::thread::spawn(move || { @@ -752,7 +698,7 @@ pub fn spawn_backend(app: &tauri::AppHandle, progress: Opt }); } - if let Some(stderr_pipe) = child.stderr.take() { + if let Some(stderr_pipe) = contained.child.stderr.take() { let app_clone = app.clone(); // Tracked (not detached): the next spawn joins this handle so this // run's buffered tail flushes before the next run's offset is taken. @@ -773,7 +719,7 @@ pub fn spawn_backend(app: &tauri::AppHandle, progress: Opt } } - Some(child) + Some(contained) } #[cfg(test)] diff --git a/frontend/src-tauri/src/bootstrap.rs b/frontend/src-tauri/src/bootstrap.rs index 45ccc80af..772a015e3 100644 --- a/frontend/src-tauri/src/bootstrap.rs +++ b/frontend/src-tauri/src/bootstrap.rs @@ -4,7 +4,7 @@ use std::fs; use std::io::{self, BufRead, BufReader}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; @@ -137,12 +137,20 @@ pub fn run_streaming( stage: &str, cmd: &mut Command, ) -> io::Result { + if backend_stop_requested(app) { + return Err(io::Error::new( + io::ErrorKind::Interrupted, + "app is quitting", + )); + } cmd.stdout(Stdio::piped()).stderr(Stdio::piped()); - // Windows: no flashing console window for uv/python subprocesses (#first-run - // terminal-window storm). No-op on macOS/Linux. stdout/stderr are piped - // above, so the splash log still receives every line. - crate::tools::no_window(cmd); - let mut child = cmd.spawn()?; + // Long installs use the same stable containment as the backend. A command + // which exits while one of its descendants is still alive is drained + // before its root handle is reaped. + let crate::tools::ContainedChild { + mut child, + mut tree, + } = crate::tools::spawn_process_tree(cmd)?; let stdout = child.stdout.take(); let stderr = child.stderr.take(); let app_out = app.clone(); @@ -165,10 +173,30 @@ pub fn run_streaming( } } }); - let status = child.wait()?; + let status = loop { + if backend_stop_requested(app) { + log::info!("App is quitting — stopping bootstrap subprocess tree (pid {})", child.id()); + break match crate::tools::terminate_process_tree( + &mut child, + &mut tree, + Duration::from_millis(750), + ) { + Ok(_) => Err(io::Error::new( + io::ErrorKind::Interrupted, + "app quit during bootstrap subprocess", + )), + Err(error) => Err(error), + }; + } + match crate::tools::contained_child_exit(&mut child, &mut tree) { + Ok(Some(status)) => break Ok(status), + Ok(None) => std::thread::sleep(Duration::from_millis(100)), + Err(error) => break Err(error), + } + }; let _ = h_out.join(); let _ = h_err.join(); - Ok(status) + status } // ── Tauri commands ──────────────────────────────────────────────────────── @@ -201,8 +229,8 @@ pub fn retry_bootstrap(app: tauri::AppHandle, state: tauri::State<'_, BootstrapS /// deletes data out from under a stopped backend and needs the *same* recovery /// afterwards — a fresh process that re-runs `ensure_dirs()` and alembic, so a /// wiped database comes back empty rather than missing. -pub fn respawn_backend( - app: tauri::AppHandle, +pub fn respawn_backend( + app: tauri::AppHandle, stage: Arc>, logs: Arc>>, ) { @@ -214,90 +242,348 @@ pub fn respawn_backend( } let stage_handle = stage; std::thread::spawn(move || { + if backend_stop_requested(&app) { + return; + } let skip_spawn = std::env::var("TAURI_SKIP_BACKEND").is_ok(); if skip_spawn { log::info!("TAURI_SKIP_BACKEND set — not spawning"); + set_backend_kill_intended(false); set_stage(&stage_handle, BootstrapStage::Ready); return; } - match crate::backend::running_backend_version(backend_port()) { - Some(v) if crate::backend::same_app_version(&v) => { - if crate::backend::backend_deep_healthy(backend_port()) { + spawn_backend_and_wait(&app, &stage_handle); + }); +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum LaunchPreparation { + SuperviseAttached { owner: u64 }, + Spawn, + Failed, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum LaunchOutcome { + SupervisedReady { owner: u64 }, + Done, +} + +/// Run a teardown while holding the same lifecycle ownership every spawn path +/// uses. Reset, Clean & Retry, setup re-entry, uninstall, and app exit join +/// launch through this guard; disk mutations keep it until they are complete, +/// so neither bootstrap nor the supervisor can resurrect the backend. +pub fn with_backend_stopped( + app: &tauri::AppHandle, + action: impl FnOnce() -> T, +) -> Result { + let state = app.state::(); + let _lifecycle = state.lifecycle.lock().unwrap_or_else(|e| e.into_inner()); + if let Err(error) = stop_backend_locked(app) { + // stop_backend_locked removes the tracked child before terminating + // it. A partial teardown error therefore cannot rely on the previous + // supervisor still existing; re-enter the serialized launch path for + // a recoverable, non-terminal caller. An incomplete tree deliberately + // retains the kill-intended fence: spawning alongside survivors could + // duplicate engines or race files still held open. + if error.restart_safe { + set_backend_kill_intended(false); + } + if error.restart_safe && !backend_stop_requested(app) { + if let Some(bootstrap) = app.try_state::() { + respawn_backend(app.clone(), bootstrap.stage.clone(), bootstrap.logs.clone()); + } + } + return Err(error.message); + } + Ok(action()) +} + +/// Stop both the tracked child (including one which has not bound yet) and any +/// untracked listener. The caller must own `BackendState::lifecycle`. +struct BackendStopError { + message: String, + restart_safe: bool, +} + +fn stop_backend_locked( + app: &tauri::AppHandle, +) -> Result<(), BackendStopError> { + set_backend_kill_intended(true); + let state = app.state::(); + let mut child = state + .process + .lock() + .unwrap_or_else(|e| e.into_inner()) + .take(); + let mut owned_tree = state + .owned_tree + .lock() + .unwrap_or_else(|e| e.into_inner()) + .take(); + let attached = state.attached.swap(false, Ordering::SeqCst); + reset_attached_health(&state); + *state + .spawned_at + .lock() + .unwrap_or_else(|e| e.into_inner()) = None; + + if attached { + if app + .try_state::() + .is_some_and(|flags| flags.quitting.load(Ordering::SeqCst)) + { + // A terminal desktop exit releases an attachment; it does not own + // the external backend and therefore must not signal it. + return Ok(()); + } + state.attached.store(true, Ordering::SeqCst); + set_backend_kill_intended(false); + return Err(BackendStopError { + message: "Another VoiceStudio instance owns the running backend. Quit that instance before changing or uninstalling its environment.".to_string(), + restart_safe: true, + }); + } + + let tree_error = match (child.as_mut(), owned_tree.as_mut()) { + (Some(child), Some(tree)) => { + // The unreaped root/process handle and containment handle move + // together, closing PID-reuse and post-crash descendant races. + log::info!("Stopping tracked backend tree (root pid {})", child.id()); + crate::tools::terminate_process_tree(child, tree, Duration::from_secs(2)).err() + } + (None, None) => None, + _ => Some(io::Error::new( + io::ErrorKind::Other, + "backend process and containment handles became inconsistent", + )), + }; + if let Some(error) = tree_error { + log::warn!("Could not fully stop tracked backend tree: {error}"); + // Preserve both stable handles for a later teardown attempt. In + // particular, never fall back to signalling the numeric root PID. + if let Some(child) = child { + *state.process.lock().unwrap_or_else(|e| e.into_inner()) = Some(child); + } + if let Some(tree) = owned_tree { + *state.owned_tree.lock().unwrap_or_else(|e| e.into_inner()) = Some(tree); + } + return Err(BackendStopError { + message: format!( + "VoiceStudio could not fully stop the backend process tree: {error}" + ), + restart_safe: false, + }); + } + + if crate::backend::port_in_use(backend_port()) + && !crate::backend::free_port_or_report(backend_port()) + { + return Err(BackendStopError { + message: format!( + "Port {} is already in use by another application, and VoiceStudio \ + could not free it. Quit whatever is using that port (another copy \ + of VoiceStudio, or an app that claimed it) and try again.", + backend_port() + ), + restart_safe: false, + }); + } + #[cfg(debug_assertions)] + if std::env::var_os("OMNIVOICE_TEST_FORCE_STOP_ERROR").is_some() { + return Err(BackendStopError { + message: "injected backend stop failure".to_string(), + restart_safe: true, + }); + } + #[cfg(debug_assertions)] + if std::env::var_os("OMNIVOICE_TEST_FORCE_INCOMPLETE_STOP_ERROR").is_some() { + return Err(BackendStopError { + message: "injected incomplete backend tree".to_string(), + restart_safe: false, + }); + } + Ok(()) +} + +fn tracked_backend_exists(app: &tauri::AppHandle) -> bool { + let state = app.state::(); + state + .process + .lock() + .unwrap_or_else(|e| e.into_inner()) + .is_some() + || state.attached.load(Ordering::SeqCst) +} + +/// Probe, attach, or reclaim the backend while lifecycle ownership is held. +/// Keeping the initial probe and any kill in the same critical section as +/// spawn+track closes the empty-port race from #1635. +fn prepare_backend_launch( + app: &tauri::AppHandle, + stage_handle: &Arc>, +) -> LaunchPreparation { + let mut replace = tracked_backend_exists(app); + match crate::backend::running_backend_version(backend_port()) { + Some(v) if crate::backend::same_app_version(&v) => { + if crate::backend::backend_deep_healthy(backend_port()) { + if replace { + let owner = SUPERVISOR_OWNER.fetch_add(1, Ordering::SeqCst) + 1; log::info!( - "Port {} already serving VoiceStudio backend v{} — attaching", - backend_port(), v + "Port {} already serving tracked VoiceStudio backend v{} — renewing supervision", + backend_port(), + v ); - set_stage(&stage_handle, BootstrapStage::Ready); - return; + set_backend_kill_intended(false); + set_stage(stage_handle, BootstrapStage::Ready); + return LaunchPreparation::SuperviseAttached { owner }; } - // Same version but a DB-touching probe fails: a backend whose - // install was wiped/corrupted while it kept running. Attaching - // would look alive and 500 on everything — replace it. - log::warn!( - "Port {} serves VoiceStudio v{} but failed the deep health probe — replacing it", - backend_port(), v + track_attached_backend(app); + let owner = SUPERVISOR_OWNER.fetch_add(1, Ordering::SeqCst) + 1; + log::info!( + "Port {} already serving VoiceStudio backend v{} — attaching with health supervision (external process remains unowned)", + backend_port(), + v ); - set_backend_kill_intended(true); // deliberate kill, not a crash (#941) - crate::backend::kill_orphan_on_port(backend_port()); - std::thread::sleep(Duration::from_millis(500)); - } - Some(v) => { - // A healthy-but-stale backend from a previous version (the - // classic post-update orphan). Attaching would silently run - // OLD backend code under the new UI — replace it instead. + set_stage(stage_handle, BootstrapStage::Ready); + return LaunchPreparation::SuperviseAttached { owner }; + } else { log::warn!( - "Port {} serves a stale VoiceStudio backend (v{} != app v{}) — replacing it", + "Port {} serves VoiceStudio v{} but failed the deep health probe — replacing it", backend_port(), - if v.is_empty() { "" } else { v.as_str() }, - env!("CARGO_PKG_VERSION"), + v ); - set_backend_kill_intended(true); // deliberate kill, not a crash (#941) - crate::backend::kill_orphan_on_port(backend_port()); - std::thread::sleep(Duration::from_millis(500)); + replace = true; } - None => {} - } - if crate::backend::port_in_use(backend_port()) { - log::warn!("Port {} in use — taking ownership", backend_port()); - set_backend_kill_intended(true); // deliberate kill, not a crash (#941) - // #1223: verify the port actually came free. Spawning into a port - // we failed to reclaim just moves the failure into the backend, - // where it surfaced as an unexplained "exit code 1". - if !crate::backend::free_port_or_report(backend_port()) { - set_stage( - &stage_handle, - BootstrapStage::Failed { - message: format!( - "Port {} is already in use by another application, \ - and VoiceStudio could not free it. Quit whatever is \ - using that port (another copy of VoiceStudio, or an \ - app that claimed it) and try again.", - backend_port() - ), - }, - ); - return; + } + Some(v) => { + log::warn!( + "Port {} serves a stale VoiceStudio backend (v{} != app v{}) — replacing it", + backend_port(), + if v.is_empty() { "" } else { v.as_str() }, + env!("CARGO_PKG_VERSION"), + ); + replace = true; + } + None => { + replace |= crate::backend::port_in_use(backend_port()); + } + } + + if replace { + log::warn!("Taking lifecycle ownership of backend port {}", backend_port()); + if let Err(message) = stop_backend_locked(app) { + if message.restart_safe { + set_backend_kill_intended(false); } + set_stage( + stage_handle, + BootstrapStage::Failed { + message: message.message, + }, + ); + return LaunchPreparation::Failed; } - spawn_backend_and_wait(&app, &stage_handle); - }); + } + LaunchPreparation::Spawn +} + +/// Initial launch preserves the setup-screen gate, but performs it only after +/// the serialized attach probe. An already-running current backend therefore +/// still wins over a missing first-run marker. +pub fn spawn_initial_backend_and_wait( + app: &tauri::AppHandle, + stage_handle: &Arc>, +) { + launch_backend_and_wait(app, stage_handle, true); } /// Spawn the backend and poll until it is healthy (→ `Ready`) or dead / -/// timed out (→ `Failed`). Shared by the launch-time bootstrap (`lib.rs`) and -/// the Retry button (`retry_bootstrap`) so both get the same recovery -/// behavior. +/// timed out (→ `Failed`). Shared by launch, Retry, reset, and setup re-entry. +pub fn spawn_backend_and_wait( + app: &tauri::AppHandle, + stage_handle: &Arc>, +) { + launch_backend_and_wait(app, stage_handle, false); +} + +fn launch_backend_and_wait( + app: &tauri::AppHandle, + stage_handle: &Arc>, + first_run_gate: bool, +) { + let outcome = { + let state = app.state::(); + let _lifecycle = state.lifecycle.lock().unwrap_or_else(|e| e.into_inner()); + if backend_stop_requested(app) { + log::info!("App is quitting — backend launch cancelled"); + LaunchOutcome::Done + } else { + match prepare_backend_launch(app, stage_handle) { + LaunchPreparation::Failed => LaunchOutcome::Done, + LaunchPreparation::SuperviseAttached { owner } => { + LaunchOutcome::SupervisedReady { owner } + } + LaunchPreparation::Spawn => { + if first_run_gate { + crate::setup::migrate_existing_install_if_needed(app); + if crate::setup::is_first_run(app) { + log::info!( + "First run — awaiting setup screen confirmation before installing" + ); + set_backend_kill_intended(false); + set_stage(stage_handle, BootstrapStage::AwaitingSetup); + LaunchOutcome::Done + } else { + spawn_with_supervisor_owner(app, stage_handle) + } + } else { + spawn_with_supervisor_owner(app, stage_handle) + } + } + } + } + }; + + if let LaunchOutcome::SupervisedReady { owner } = outcome { + supervise_backend(app, stage_handle, owner); + } +} + +/// Invalidate any previous supervisor before creating a replacement child, so +/// it cannot mistake this child's pre-Ready exit for a post-Ready crash. The +/// reserved owner starts monitoring only after this launch reaches Ready. +fn spawn_with_supervisor_owner( + app: &tauri::AppHandle, + stage_handle: &Arc>, +) -> LaunchOutcome { + let supervisor_owner = SUPERVISOR_OWNER.fetch_add(1, Ordering::SeqCst) + 1; + if spawn_backend_until_ready(app, stage_handle) { + LaunchOutcome::SupervisedReady { + owner: supervisor_owner, + } + } else { + LaunchOutcome::Done + } +} + /// /// #314: when the backend dies with a broken-venv signature ("No pyvenv.cfg /// file" / exit code 106 from the CPython venv launcher), the venv — and only /// the venv — is removed and the bootstrap re-runs once, recreating it through /// the normal `CreatingVenv` / `InstallingDeps` setup path instead of -/// surfacing the same dead-end failure on every retry. -pub fn spawn_backend_and_wait(app: &tauri::AppHandle, stage_handle: &Arc>) { +/// surfacing the same dead-end failure on every retry. Lifecycle ownership is +/// already held by the caller for this entire function. +fn spawn_backend_until_ready( + app: &tauri::AppHandle, + stage_handle: &Arc>, +) -> bool { let mut venv_heal_attempted = false; 'bootstrap: loop { - let child = crate::backend::spawn_backend(app, Some(stage_handle)); - track_backend_child(app, child); + if backend_stop_requested(app) { + return false; + } + spawn_and_track_backend(app, stage_handle); let start = std::time::Instant::now(); // Early-bind narration: the backend answers /startup/progress within // ~1s of spawn, long before it is Ready — surface each step change @@ -306,42 +592,35 @@ pub fn spawn_backend_and_wait(app: &tauri::AppHandle, stag // None and the wait looks exactly as it did before. let mut last_step = String::new(); while start.elapsed() < startup_budget() { + if backend_stop_requested(app) { + log::info!("App is quitting — backend startup poll cancelled"); + return false; + } if crate::backend::backend_ready(backend_port()) { set_stage(stage_handle, BootstrapStage::Ready); - // #567/#570/#571: once Ready, keep watching the backend child - // and respawn it if it dies mid-session, so a crash self-heals - // instead of leaving every later request to dead-end on - // "Can't reach the local backend". Only one supervisor runs at - // a time — Retry can re-enter this function concurrently. - if SUPERVISOR_ACTIVE - .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) - .is_ok() - { - supervise_backend(app, stage_handle); - SUPERVISOR_ACTIVE.store(false, Ordering::SeqCst); - } - return; + return true; } let process_dead: Option<(String, Option)> = - if let Ok(mut guard) = app.state::().process.lock() { - match guard.as_mut() { - Some(child) => match child.try_wait() { - Ok(Some(status)) => { - let exit = BackendExit::from_status(status); - Some((exit.description.clone(), Some(exit))) - } - Ok(None) => None, - // try_wait errored — the death is real but its - // shape is unknown; no exit code for the marker. - Err(_) => Some(("unknown".to_string(), None)), - }, + match backend_child_exit(app) { + Ok(Some(exit)) => Some((exit.description.clone(), Some(exit))), + Ok(None) if !tracked_backend_exists(app) => { // Spawn itself failed — no process ever ran, so this // is a spawn failure (spawn_failure_diagnostic owns // it), NOT a crash: no marker. - None => Some(("never started".to_string(), None)), + Some(("never started".to_string(), None)) + } + Ok(None) => None, + Err(error) => { + // The root was observed but its stable containment + // could not be drained. Never start a second backend + // alongside descendants whose teardown is uncertain. + set_backend_kill_intended(true); + let message = format!( + "VoiceStudio could not safely clean up the failed backend: {error}" + ); + set_stage(stage_handle, BootstrapStage::Failed { message }); + return false; } - } else { - None }; if let Some((exit_info, real_exit)) = process_dead { let err_tail = crate::backend::read_error_log_tail_for_run(30); @@ -349,7 +628,7 @@ pub fn spawn_backend_and_wait(app: &tauri::AppHandle, stag // startup crashes included — unless the app is shutting down // or a retry flow deliberately killed the child. if let Some(ref exit) = real_exit { - if !app_is_quitting(app) && !backend_kill_intended() { + if !backend_stop_requested(app) && !backend_kill_intended() { crate::crash::record_crash(crate::crash::marker_now( exit, backend_uptime_s(app), @@ -419,7 +698,7 @@ pub fn spawn_backend_and_wait(app: &tauri::AppHandle, stag "Backend never started ({}) — keeping the specific failure already diagnosed", exit_info ); - return; + return false; } // #1223: the backend exits EXIT_PORT_IN_USE when it could not // bind its port. That is a conflict, not a crash — say what to @@ -445,7 +724,7 @@ pub fn spawn_backend_and_wait(app: &tauri::AppHandle, stag }; log::error!("Backend died early: {}", msg); set_stage(stage_handle, BootstrapStage::Failed { message: msg }); - return; + return false; } if let Some((status, step, label)) = crate::backend::startup_progress(backend_port()) @@ -457,6 +736,9 @@ pub fn spawn_backend_and_wait(app: &tauri::AppHandle, stag } std::thread::sleep(Duration::from_millis(500)); } + if backend_stop_requested(app) { + return false; + } let err_tail = crate::backend::read_error_log_tail_for_run(20); let msg = if err_tail.is_empty() { format!("Backend did not respond within {} s", startup_budget().as_secs()) @@ -468,7 +750,7 @@ pub fn spawn_backend_and_wait(app: &tauri::AppHandle, stag ) }; set_stage(stage_handle, BootstrapStage::Failed { message: msg }); - return; + return false; } } @@ -483,10 +765,11 @@ pub fn spawn_backend_and_wait(app: &tauri::AppHandle, stag // app. The supervisor closes that gap: after Ready, it watches the child and // respawns it (bounded) so a crash self-heals. -/// Only one supervisor loop may run at a time. The launch-time bootstrap and -/// the Retry button both call `spawn_backend_and_wait` (and can race), so the -/// first to reach Ready claims this and the rest fall through. -static SUPERVISOR_ACTIVE: AtomicBool = AtomicBool::new(false); +/// Ownership token for the supervisor loop. Each lifecycle-owned spawn +/// reserves a new token before creating its child, then starts monitoring only +/// if that child reaches Ready. An older loop observes the mismatch before any +/// later lifecycle mutation and exits, without a timing-based handoff. +static SUPERVISOR_OWNER: AtomicU64 = AtomicU64::new(0); /// #941: set while a retry/clean-retry flow deliberately kills the backend to /// replace it, so the death watchers (startup poll + supervisor) never write a @@ -511,6 +794,13 @@ fn backend_kill_intended() -> bool { BACKEND_KILL_INTENDED.load(Ordering::SeqCst) } +/// Whether a failed stop completed enough teardown for an explicit recovery +/// launch. Incomplete process trees keep the deliberate-kill fence raised so +/// uninstall rollback cannot spawn alongside surviving engines/installers. +pub fn backend_stop_recovery_safe() -> bool { + !backend_kill_intended() +} + /// How much of backend_err.log rides inside a crash marker (#941). ~40 lines /// is enough for a Python traceback or a native abort banner without bloating /// the marker file or the bug-report URL (the frontend truncates further). @@ -526,27 +816,95 @@ const CRASH_STDERR_TAIL_LINES: usize = 40; const MAX_RESTARTS: usize = 3; const RESTART_WINDOW: Duration = Duration::from_secs(600); -fn app_is_quitting(app: &tauri::AppHandle) -> bool { +fn backend_stop_requested(app: &tauri::AppHandle) -> bool { app.try_state::() - .map(|f| f.quitting.load(Ordering::SeqCst)) + .map(|f| f.quitting.load(Ordering::SeqCst) || f.uninstalling.load(Ordering::SeqCst)) .unwrap_or(false) } /// Store the freshly spawned backend child (and its spawn time, for the crash /// marker's `uptime_s`), and re-arm the death watchers: any deliberate-kill /// window ends the moment a new child is tracked. -fn track_backend_child(app: &tauri::AppHandle, child: Option) { +fn track_backend_child( + app: &tauri::AppHandle, + contained: Option, +) { let state = app.state::(); - if let Ok(mut guard) = state.process.lock() { - *guard = child; - } - if let Ok(mut spawned) = state.spawned_at.lock() { - *spawned = Some(Instant::now()); - } + let (child, owned_tree, spawned_at) = match contained { + Some(crate::tools::ContainedChild { child, tree }) => { + (Some(child), Some(tree), Some(Instant::now())) + } + None => (None, None, None), + }; + *state.process.lock().unwrap_or_else(|e| e.into_inner()) = child; + *state.owned_tree.lock().unwrap_or_else(|e| e.into_inner()) = owned_tree; + *state.spawned_at.lock().unwrap_or_else(|e| e.into_inner()) = spawned_at; + state.attached.store(false, Ordering::SeqCst); + reset_attached_health(&state); BACKEND_SPAWN_GENERATION.fetch_add(1, Ordering::SeqCst); set_backend_kill_intended(false); } +/// Health-supervise a same-version listener which predates this launch. It is +/// intentionally not adopted by PID: only a process spawned into our stable +/// containment primitive is safe for this desktop instance to terminate. +fn track_attached_backend(app: &tauri::AppHandle) { + let state = app.state::(); + *state.process.lock().unwrap_or_else(|e| e.into_inner()) = None; + *state.owned_tree.lock().unwrap_or_else(|e| e.into_inner()) = None; + *state + .spawned_at + .lock() + .unwrap_or_else(|e| e.into_inner()) = None; + state.attached.store(true, Ordering::SeqCst); + reset_attached_health(&state); + BACKEND_SPAWN_GENERATION.fetch_add(1, Ordering::SeqCst); + set_backend_kill_intended(false); +} + +/// The one spawn→track chokepoint. Callers already hold lifecycle ownership, +/// so no child can be created without becoming the uniquely tracked child. +fn spawn_and_track_backend( + app: &tauri::AppHandle, + stage_handle: &Arc>, +) { + let child = crate::backend::spawn_backend(app, Some(stage_handle)); + #[cfg(debug_assertions)] + wait_for_tracking_test_gate( + "OMNIVOICE_TEST_BEFORE_TRACK_ENTERED", + "OMNIVOICE_TEST_BEFORE_TRACK_RELEASE", + ); + track_backend_child(app, child); + #[cfg(debug_assertions)] + wait_for_tracking_test_gate( + "OMNIVOICE_TEST_AFTER_TRACK_ENTERED", + "OMNIVOICE_TEST_AFTER_TRACK_RELEASE", + ); +} + +/// Deterministic fault-injection seam around child tracking. These are the +/// precise pre-bind shutdown/uninstall races that process-only or port-only +/// teardown used to lose. Compiled out of release builds. +#[cfg(debug_assertions)] +fn wait_for_tracking_test_gate(entered_var: &str, release_var: &str) { + let Some(entered) = std::env::var_os(entered_var) else { + return; + }; + let Some(release) = std::env::var_os(release_var) else { + return; + }; + let entered = std::path::PathBuf::from(entered); + let release = std::path::PathBuf::from(release); + if let Err(error) = std::fs::write(&entered, b"spawned") { + log::warn!("Could not arm lifecycle test seam {entered_var}: {error}"); + return; + } + let deadline = Instant::now() + Duration::from_secs(10); + while !release.exists() && Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(10)); + } +} + /// Seconds since the tracked backend child was spawned (0 when unknown). fn backend_uptime_s(app: &tauri::AppHandle) -> u64 { app.try_state::() @@ -555,20 +913,90 @@ fn backend_uptime_s(app: &tauri::AppHandle) -> u64 { .unwrap_or(0) } -/// Returns `Some(BackendExit)` if the tracked backend child has exited, -/// `None` if it is still running (or none is tracked — which we never treat as -/// a death to respawn, to avoid fighting a deliberate teardown). -fn backend_child_exit(app: &tauri::AppHandle) -> Option { - let state = app.try_state::()?; - let mut guard = state.process.lock().ok()?; - match guard.as_mut() { - Some(child) => match child.try_wait() { - Ok(Some(status)) => Some(BackendExit::from_status(status)), - Ok(None) => None, - Err(e) => Some(BackendExit::unknown(&format!("try_wait error: {e}"))), - }, - None => None, +/// Observe a contained child through its stable root/containment handles, or +/// health-monitor an external attachment without ever signalling it. +fn backend_child_exit( + app: &tauri::AppHandle, +) -> Result, String> { + let Some(state) = app.try_state::() else { + return Ok(None); + }; + let mut process = state.process.lock().map_err(|e| e.to_string())?; + if let Some(child) = process.as_mut() { + let mut tree = state.owned_tree.lock().map_err(|e| e.to_string())?; + let owned = tree + .as_mut() + .ok_or_else(|| "tracked backend is missing its containment handle".to_string())?; + return match crate::tools::contained_child_exit(child, owned) { + Ok(Some(status)) => { + *process = None; + *tree = None; + Ok(Some(BackendExit::from_status(status))) + } + Ok(None) => Ok(None), + Err(error) => Err(format!( + "could not clean the contained backend after observing its root: {error}" + )), + }; } + if state.attached.load(Ordering::SeqCst) { + if attached_backend_healthy() { + reset_attached_health(&state); + return Ok(None); + } + if !attached_outage_confirmed(&state) { + return Ok(None); + } + // The grace window elapsed; take one final independent sample before + // changing ownership state. A recovered external backend is still + // unowned and must remain attached, never signalled by PID. + if attached_backend_healthy() { + reset_attached_health(&state); + return Ok(None); + } + return Ok(Some(BackendExit::unknown( + "attached backend stopped responding after the health grace period", + ))); + } + Ok(None) +} + +const ATTACHED_FAILURE_THRESHOLD: u32 = 3; + +fn attached_backend_healthy() -> bool { + crate::backend::running_backend_version(backend_port()) + .is_some_and(|version| crate::backend::same_app_version(&version)) + && crate::backend::backend_deep_healthy(backend_port()) +} + +fn attached_health_grace() -> Duration { + std::env::var("OMNIVOICE_ATTACHED_FAILURE_GRACE_MS") + .ok() + .and_then(|value| value.trim().parse::().ok()) + .filter(|&millis| millis > 0) + .map(Duration::from_millis) + .unwrap_or(Duration::from_secs(6)) +} + +fn reset_attached_health(state: &BackendState) { + let mut health = state + .attached_health + .lock() + .unwrap_or_else(|error| error.into_inner()); + health.failures = 0; + health.unhealthy_since = None; +} + +fn attached_outage_confirmed(state: &BackendState) -> bool { + let now = Instant::now(); + let mut health = state + .attached_health + .lock() + .unwrap_or_else(|error| error.into_inner()); + health.failures = health.failures.saturating_add(1); + let since = *health.unhealthy_since.get_or_insert(now); + health.failures >= ATTACHED_FAILURE_THRESHOLD + && now.duration_since(since) >= attached_health_grace() } /// How long the launch poll waits for the backend to become Ready before @@ -620,14 +1048,17 @@ fn restart_backoff_delay(recent_restarts: usize) -> Duration { /// After the backend is Ready, watch its process and respawn it on an /// unexpected exit. Runs on the (otherwise-returning) bootstrap thread and /// stops the instant the app is quitting so it never resurrects the backend -/// during shutdown. Death is detected only via a *confirmed process exit* -/// (`try_wait`), never a slow health probe, so a busy-but-alive backend is -/// never killed. -fn supervise_backend(app: &tauri::AppHandle, stage_handle: &Arc>) { +/// during shutdown. A desktop-spawned backend is observed through its stable +/// child handle; a pre-existing attachment is health-probed but never killed. +fn supervise_backend( + app: &tauri::AppHandle, + stage_handle: &Arc>, + owner: u64, +) { let mut restart_times: Vec = Vec::new(); loop { std::thread::sleep(supervisor_poll()); - if app_is_quitting(app) { + if backend_stop_requested(app) || SUPERVISOR_OWNER.load(Ordering::SeqCst) != owner { return; } // Snapshot the spawn generation BEFORE observing the exit: sampled @@ -637,12 +1068,27 @@ fn supervise_backend(app: &tauri::AppHandle, stage_handle: // happens from here on — even one whose child we are about to see // exit — reads as a generation change and yields. let observed_generation = BACKEND_SPAWN_GENERATION.load(Ordering::SeqCst); + let backend_state = app.state::(); + let was_attached = backend_state.attached.load(Ordering::SeqCst); + let mut attachment_lifecycle = None; let exit = match backend_child_exit(app) { - Some(exit) => exit, - None => continue, // still running + Ok(Some(exit)) => exit, + Ok(None) => continue, // still running + Err(error) => { + set_backend_kill_intended(true); + let message = format!( + "VoiceStudio could not safely clean up the crashed backend: {error}" + ); + log::error!("{message}"); + set_stage(stage_handle, BootstrapStage::Failed { message }); + return; + } }; // The exit may have raced with a shutdown that killed the child. - if app_is_quitting(app) { + if backend_stop_requested(app) { + return; + } + if SUPERVISOR_OWNER.load(Ordering::SeqCst) != owner { return; } // A retry/clean-retry flow killed the child on purpose and owns the @@ -652,36 +1098,74 @@ fn supervise_backend(app: &tauri::AppHandle, stage_handle: log::info!("Backend exit was a deliberate replace — supervisor yielding to the retry flow"); return; } + if was_attached { + // An unhealthy response is not proof that this unowned process + // died. Serialize a final recovery/port sample with lifecycle + // owners. If it recovered, keep supervising it; if it still owns + // the port, re-arm observation and wait without signalling it or + // landing permanently in Failed. Only a released port permits a + // desktop-owned replacement. + let lifecycle = backend_state + .lifecycle + .lock() + .unwrap_or_else(|e| e.into_inner()); + if backend_stop_requested(app) + || SUPERVISOR_OWNER.load(Ordering::SeqCst) != owner + || BACKEND_SPAWN_GENERATION.load(Ordering::SeqCst) != observed_generation + { + return; + } + if attached_backend_healthy() { + track_attached_backend(app); + set_stage(stage_handle, BootstrapStage::Ready); + continue; + } + if crate::backend::port_in_use(backend_port()) { + backend_state.attached.store(true, Ordering::SeqCst); + continue; + } + backend_state.attached.store(false, Ordering::SeqCst); + // Keep ownership from the final free-port observation through the + // spawn-and-track handoff below; no internal lifecycle flow can + // interleave and no external listener is ever signalled. + attachment_lifecycle = Some(lifecycle); + } let exit_info = exit.description.clone(); // #941: make the death self-documenting BEFORE any restart attempt — // the marker (exit code/signal + stderr tail + uptime) is what turns // the next "Can't reach the backend" report into a diagnosable one. - let uptime_s = backend_uptime_s(app); - crate::crash::record_crash(crate::crash::marker_now( - &exit, - uptime_s, - crate::backend::read_error_log_tail_for_run(CRASH_STDERR_TAIL_LINES), - )); - if restart_budget_exhausted(&mut restart_times, Instant::now()) { - let tail = crate::backend::read_error_log_tail_for_run(30); - let msg = format!( - "The backend kept crashing ({} times in {} min; last death: {}) and couldn't \ - be kept running. Use Clean & Retry, or check Settings → Logs → Backend.{}", - MAX_RESTARTS, - RESTART_WINDOW.as_secs() / 60, - exit.label(), - if tail.is_empty() { String::new() } else { format!("\n\nLast output:\n{tail}") }, - ); - log::error!("Backend supervisor giving up: {msg}"); - let _ = app.emit("backend-restart-failed", msg.clone()); - set_stage(stage_handle, BootstrapStage::Failed { message: msg }); - return; - } - // Backoff BEFORE this restart is recorded: `restart_times` was just - // pruned to the window, so its length is the number of recent - // respawns already attempted. - let backoff = restart_backoff_delay(restart_times.len()); - restart_times.push(Instant::now()); + let backoff = if was_attached { + // A health-confirmed disconnect from an unowned listener is not a + // process crash: no fabricated crash marker/budget entry. + Duration::ZERO + } else { + let uptime_s = backend_uptime_s(app); + crate::crash::record_crash(crate::crash::marker_now( + &exit, + uptime_s, + crate::backend::read_error_log_tail_for_run(CRASH_STDERR_TAIL_LINES), + )); + if restart_budget_exhausted(&mut restart_times, Instant::now()) { + let tail = crate::backend::read_error_log_tail_for_run(30); + let msg = format!( + "The backend kept crashing ({} times in {} min; last death: {}) and couldn't \ + be kept running. Use Clean & Retry, or check Settings → Logs → Backend.{}", + MAX_RESTARTS, + RESTART_WINDOW.as_secs() / 60, + exit.label(), + if tail.is_empty() { String::new() } else { format!("\n\nLast output:\n{tail}") }, + ); + log::error!("Backend supervisor giving up: {msg}"); + let _ = app.emit("backend-restart-failed", msg.clone()); + set_stage(stage_handle, BootstrapStage::Failed { message: msg }); + return; + } + // Backoff BEFORE this restart is recorded: `restart_times` was + // just pruned, so its length is recent respawns already attempted. + let delay = restart_backoff_delay(restart_times.len()); + restart_times.push(Instant::now()); + delay + }; log::warn!("Backend process exited unexpectedly ({exit_info}) — restarting it (#567)"); emit_log(app, "starting_backend", "Backend stopped unexpectedly — restarting it automatically"); // Frontend listens for this to show a "reconnecting" banner (the splash @@ -700,7 +1184,10 @@ fn supervise_backend(app: &tauri::AppHandle, stage_handle: ); let waited = Instant::now(); while waited.elapsed() < backoff { - if app_is_quitting(app) { + if backend_stop_requested(app) { + return; + } + if SUPERVISOR_OWNER.load(Ordering::SeqCst) != owner { return; } if backend_kill_intended() { @@ -714,9 +1201,8 @@ fn supervise_backend(app: &tauri::AppHandle, stage_handle: // when a replacement is tracked and never un-bumps, so it is // observed even if the replacement has itself already exited // by the time we sample. Yield promptly (not at backoff end) - // so the retry's own spawn_backend_and_wait can claim the - // supervisor slot at Ready — and so we never free_port() a - // replacement out from under the flow that owns it. + // so the replacement flow can claim the supervisor slot, and + // never free_port() its child out from under it. if BACKEND_SPAWN_GENERATION.load(Ordering::SeqCst) != observed_generation { log::info!( "A replacement backend was tracked during restart backoff — supervisor yielding" @@ -726,9 +1212,30 @@ fn supervise_backend(app: &tauri::AppHandle, stage_handle: std::thread::sleep(Duration::from_millis(500)); } } + // Claim the same lifecycle ownership used by bootstrap/Retry before + // the final probe. Another flow may have completed a whole replacement + // between our backoff samples; after this lock, probe/kill/spawn/track + // stay atomic with respect to every other owner (#1635). + let _lifecycle = match attachment_lifecycle { + Some(lifecycle) => lifecycle, + None => backend_state + .lifecycle + .lock() + .unwrap_or_else(|e| e.into_inner()), + }; + if backend_stop_requested(app) { + return; + } + if SUPERVISOR_OWNER.load(Ordering::SeqCst) != owner { + return; + } + if backend_kill_intended() { + log::info!("Deliberate replace owns the backend lifecycle — supervisor yielding"); + return; + } // Last look before touching the port — covers the zero-backoff first - // respawn (which never enters the pause loop) and the tail of the - // pause itself. After this point we own the respawn. + // respawn and a replacement completed while this supervisor waited for + // lifecycle ownership. if BACKEND_SPAWN_GENERATION.load(Ordering::SeqCst) != observed_generation { log::info!("A replacement backend was tracked — supervisor yielding to its flow"); return; @@ -759,14 +1266,13 @@ fn supervise_backend(app: &tauri::AppHandle, stage_handle: ); return; } - let child = crate::backend::spawn_backend(app, Some(stage_handle)); - track_backend_child(app, child); + spawn_and_track_backend(app, stage_handle); // Wait (bounded) for the respawn to become healthy. If it dies again // immediately, bail early so the next loop counts it toward the cap. let start = Instant::now(); let mut last_step = String::new(); while start.elapsed() < Duration::from_secs(120) { - if app_is_quitting(app) { + if backend_stop_requested(app) { return; } if crate::backend::backend_ready(backend_port()) { @@ -775,8 +1281,17 @@ fn supervise_backend(app: &tauri::AppHandle, stage_handle: log::info!("Backend restarted and healthy again"); break; } - if backend_child_exit(app).is_some() { - break; + match backend_child_exit(app) { + Ok(Some(_)) => break, + Ok(None) => {} + Err(error) => { + set_backend_kill_intended(true); + let message = format!( + "VoiceStudio could not safely clean up the restarted backend: {error}" + ); + set_stage(stage_handle, BootstrapStage::Failed { message }); + return; + } } // Same early-bind narration as the launch poll: name the startup // step in the reconnecting window instead of a silent wait. @@ -794,23 +1309,39 @@ fn supervise_backend(app: &tauri::AppHandle, stage_handle: } #[tauri::command] -pub fn clean_and_retry_bootstrap(app: tauri::AppHandle, state: tauri::State<'_, BootstrapState>) { - // env_root honors the setup-screen choice (portable / custom env dir), so - // clean-retry removes the venv the bootstrap actually uses. - let project_dir = crate::setup::env_root(&app).join("project"); - if project_dir.is_dir() { - log::info!("Clean retry: removing {}", project_dir.display()); - let _ = fs::remove_dir_all(&project_dir); - } - // Kill any zombie backend still occupying the port from the deleted - // project dir, otherwise bootstrap will "attach" to the stale process. - if crate::backend::port_in_use(backend_port()) { - log::warn!("Clean retry: killing stale backend on port {}", backend_port()); - set_backend_kill_intended(true); // deliberate kill, not a crash (#941) - crate::backend::kill_orphan_on_port(backend_port()); - std::thread::sleep(Duration::from_millis(500)); - } - retry_bootstrap(app, state); +pub async fn clean_and_retry_bootstrap(app: tauri::AppHandle) { + let state = app.state::(); + let failure_stage = state.stage.clone(); + let worker_app = app.clone(); + let worker_stage = state.stage.clone(); + let worker_logs = state.logs.clone(); + let joined = tauri::async_runtime::spawn_blocking(move || { + // env_root honors the setup-screen choice (portable / custom env dir), + // so clean-retry removes the venv the bootstrap actually uses. Stop, + // recursive deletion, and recovery all stay in this retained blocking + // task: the UI thread never waits on process teardown or filesystem I/O, + // and navigation cannot drop the respawn handoff. + let project_dir = crate::setup::env_root(&worker_app).join("project"); + match with_backend_stopped(&worker_app, || { + if project_dir.is_dir() { + log::info!("Clean retry: removing {}", project_dir.display()); + let _ = fs::remove_dir_all(&project_dir); + } + }) { + Ok(()) => respawn_backend(worker_app, worker_stage, worker_logs), + Err(message) => set_stage(&worker_stage, BootstrapStage::Failed { message }), + } + }) + .await; + if let Err(error) = joined { + log::error!("Clean & Retry task failed to join: {error}"); + set_stage( + &failure_stage, + BootstrapStage::Failed { + message: "Clean & Retry task failed unexpectedly".to_string(), + }, + ); + } } // ── Venv bootstrap ──────────────────────────────────────────────────────── diff --git a/frontend/src-tauri/src/lib.rs b/frontend/src-tauri/src/lib.rs index 7bc5fed57..bfc392d62 100644 --- a/frontend/src-tauri/src/lib.rs +++ b/frontend/src-tauri/src/lib.rs @@ -28,7 +28,6 @@ use std::collections::VecDeque; use std::process::Child; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; -use std::time::Duration; use tauri::menu::{MenuBuilder, MenuItemBuilder}; use tauri::tray::TrayIconBuilder; @@ -52,14 +51,44 @@ pub fn backend_port() -> u16 { // ── Shared state types ──────────────────────────────────────────────────── pub struct BackendState { + /// Serializes every desktop lifecycle owner from its first port/process + /// probe through child tracking and readiness. Bootstrap, Retry, reset, + /// setup/uninstall, shutdown, and the crash supervisor never overlap. + pub lifecycle: Mutex<()>, pub process: Mutex>, + /// Stable OS containment for a desktop-spawned backend. Unix keeps the + /// root unreaped until its inherited process group is drained; Windows + /// owns a kill-on-close Job handle. Neither path signals a reusable PID. + pub owned_tree: Mutex>, + /// A healthy same-version backend which predates this desktop launch. + /// It is health-supervised but deliberately never killed by PID: there is + /// no safe way to adopt ownership of an arbitrary external process tree. + pub attached: AtomicBool, + /// Consecutive deep-health failures for an unowned attachment. A single + /// busy/slow response is not process death; the supervisor confirms a + /// sustained outage before considering a safe replacement. + pub attached_health: Mutex, /// When the tracked child was spawned — feeds the crash marker's /// `uptime_s` (#941). Set alongside `process` in bootstrap.rs. pub spawned_at: Mutex>, } +#[derive(Default)] +pub struct AttachedHealthState { + pub failures: u32, + pub unhealthy_since: Option, +} + pub struct AppFlags { pub quitting: AtomicBool, + /// A destructive uninstall is stopping the backend but is not itself a + /// terminal app exit until the purge succeeds. Keeping this separate from + /// `quitting` lets CloseRequested keep the main window alive and lets a + /// failed purge recover without overwriting a concurrent real exit. + pub uninstalling: AtomicBool, + /// Generation which owns `uninstalling`. A stale join/panic finalizer may + /// only release its own claim, never a newer uninstall attempt. + pub uninstall_owner: std::sync::atomic::AtomicU64, /// Whether dictation is currently recording. The tray's Start/Stop item /// used to infer this from `widget.is_visible()`, which stopped meaning /// anything once the widget became a permanently hidden host. The frontend @@ -453,6 +482,22 @@ mod pill_noactivate_tests { // ── Tauri entry ─────────────────────────────────────────────────────────── +/// Production `ExitRequested` teardown, exposed so the real-child lifecycle +/// harness exercises the exact shutdown path used by the desktop event loop. +#[doc(hidden)] +pub fn shutdown_backend_for_exit(app_handle: &tauri::AppHandle) { + // Raise the quitting flag FIRST: exits that don't pass through the tray + // Quit item (macOS ⌘Q, OS session end) would otherwise let a death watcher + // observe our own termination and record a false crash marker (#941). + app_handle + .state::() + .quitting + .store(true, Ordering::SeqCst); + if let Err(error) = bootstrap::with_backend_stopped(app_handle, || {}) { + log::warn!("Could not fully stop the backend during app exit: {error}"); + } +} + #[cfg_attr(mobile, tauri::mobile_entry_point)] pub fn run() { // #879: if the previous run requested a WebView cache repair (splash @@ -672,6 +717,8 @@ pub fn run() { app.manage(AppFlags { quitting: AtomicBool::new(false), + uninstalling: AtomicBool::new(false), + uninstall_owner: std::sync::atomic::AtomicU64::new(0), dictating: AtomicBool::new(false), capture: Mutex::new(CaptureDispatchState::default()), output: dictation_output::DictationOutput::default(), @@ -952,7 +999,11 @@ pub fn run() { let stage_handle = bootstrap_state.stage.clone(); app.manage(bootstrap_state); app.manage(BackendState { + lifecycle: Mutex::new(()), process: Mutex::new(None), + owned_tree: Mutex::new(None), + attached: AtomicBool::new(false), + attached_health: Mutex::new(AttachedHealthState::default()), spawned_at: Mutex::new(None), }); @@ -972,68 +1023,11 @@ pub fn run() { set_stage(&stage_handle, BootstrapStage::AwaitingSetup); return; } - match backend::running_backend_version(backend_port()) { - Some(v) if backend::same_app_version(&v) => { - if backend::backend_deep_healthy(backend_port()) { - log::info!( - "Port {} already serving VoiceStudio backend v{} — attaching", - backend_port(), v - ); - set_stage(&stage_handle, BootstrapStage::Ready); - return; - } - // Same version but a DB-touching probe fails: a backend whose - // install was wiped/corrupted while it kept running. Attaching - // would look alive and 500 on everything — replace it. - log::warn!( - "Port {} serves VoiceStudio v{} but failed the deep health probe — replacing it", - backend_port(), v - ); - backend::kill_orphan_on_port(backend_port()); - std::thread::sleep(Duration::from_millis(500)); - } - Some(v) => { - // Healthy-but-stale backend from a previous version — - // the post-update orphan that made new installs run - // old backend code. Replace it (see backend.rs - // same_app_version for the full story). - log::warn!( - "Port {} serves a stale VoiceStudio backend (v{} != app v{}) — replacing it", - backend_port(), - if v.is_empty() { "" } else { v.as_str() }, - env!("CARGO_PKG_VERSION"), - ); - backend::kill_orphan_on_port(backend_port()); - std::thread::sleep(Duration::from_millis(500)); - } - None => {} - } - if backend::port_in_use(backend_port()) { - log::warn!( - "Port {} in use — taking ownership (killing whatever's there)", - backend_port() - ); - backend::kill_orphan_on_port(backend_port()); - std::thread::sleep(Duration::from_millis(500)); - } - // First-run gate: never auto-install. With nothing on disk to - // attach to, park on the setup screen and wait for the user to - // confirm an install plan — `complete_setup` restarts the - // bootstrap from there. Existing pre-setup-screen installs - // (venv present) are migrated here — the bootstrap thread is - // the only place that write happens — then pass straight - // through the read-only is_first_run check. - setup::migrate_existing_install_if_needed(&app_handle); - if setup::is_first_run(&app_handle) { - log::info!("First run — awaiting setup screen confirmation before installing"); - set_stage(&stage_handle, BootstrapStage::AwaitingSetup); - return; - } - // Spawn + health-poll loop shared with the Retry button — - // includes the #314 broken-venv self-heal (quarantine the - // venv and rebuild once when the backend exits with - // "No pyvenv.cfg file" / code 106). - bootstrap::spawn_backend_and_wait(&app_handle, &stage_handle); + // Probe/attach, the first-run gate, spawn, child tracking, and + // readiness are one serialized lifecycle operation. A Retry + // arriving during launch waits and attaches instead of + // creating a second backend on the same port (#1635). + bootstrap::spawn_initial_backend_and_wait(&app_handle, &stage_handle); }); Ok(()) }) @@ -1066,46 +1060,7 @@ pub fn run() { if !persistence_exit::handle_exit_requested(app_handle, code, &api) { return; } - // Raise the quitting flag FIRST: exits that don't pass through the - // tray Quit item (macOS ⌘Q, OS session end) would otherwise let a - // death watcher observe our own SIGTERM below and record a false - // "backend crashed" marker (#941). - app_handle - .state::() - .quitting - .store(true, Ordering::SeqCst); - if let Ok(mut lock) = app_handle.state::().process.lock() { - if let Some(ref mut child) = *lock { - let pid = child.id(); - log::info!("Shutting down backend (pid {})", pid); - - #[cfg(unix)] - { - unsafe { - libc::kill(pid as i32, libc::SIGTERM); - } - let start = std::time::Instant::now(); - loop { - match child.try_wait() { - Ok(Some(_)) => break, - Ok(None) if start.elapsed() < Duration::from_secs(2) => { - std::thread::sleep(Duration::from_millis(100)); - } - _ => { - log::warn!("Backend didn't exit in 2 s — SIGKILL"); - let _ = child.kill(); - break; - } - } - } - } - #[cfg(not(unix))] - { - let _ = child.kill(); - } - let _ = child.wait(); - } - } + shutdown_backend_for_exit(app_handle); } }); } diff --git a/frontend/src-tauri/src/reset.rs b/frontend/src-tauri/src/reset.rs index 98e138a56..838756f86 100644 --- a/frontend/src-tauri/src/reset.rs +++ b/frontend/src-tauri/src/reset.rs @@ -40,7 +40,7 @@ use serde::Serialize; use tauri::Manager; use crate::bootstrap::BootstrapState; -use crate::{backend_port, AppFlags}; +use crate::AppFlags; /// Every scope the UI can offer. Two of them (`ui_prefs`, `history`) own no /// files — they are listed here so the frontend has one registry to render, but @@ -374,6 +374,14 @@ pub fn purge_scopes(roots: &Roots, wanted: &[String], home: Option<&Path>) -> Re report } +async fn run_retained_reset( + worker: impl FnOnce() -> Result + Send + 'static, +) -> Result { + tauri::async_runtime::spawn_blocking(worker) + .await + .map_err(|error| format!("reset failed: {error}"))? +} + /// Delete the selected scopes, then bring the backend back. /// /// Unknown or frontend-only scope names are ignored rather than erroring: the @@ -391,45 +399,85 @@ pub async fn reset_purge(app: tauri::AppHandle, scopes: Vec) -> Result(); + if !flags.quitting.load(std::sync::atomic::Ordering::SeqCst) + && !flags.uninstalling.load(std::sync::atomic::Ordering::SeqCst) + { + let state = purge_app.state::(); + crate::bootstrap::respawn_backend( + purge_app.clone(), + state.stage.clone(), + state.logs.clone(), + ); + report.restarted = true; + } else { + crate::bootstrap::set_backend_kill_intended(false); + } + Ok(report) }) .await - .map_err(|e| format!("reset failed: {e}"))?; - - // Back up. The fresh backend re-runs ensure_dirs() and alembic, so a deleted - // database returns empty instead of missing. If the app is on its way out - // anyway, don't fight the shutdown. - let flags = app.state::(); - if !flags.quitting.load(std::sync::atomic::Ordering::SeqCst) { - let state = app.state::(); - crate::bootstrap::respawn_backend(app.clone(), state.stage.clone(), state.logs.clone()); - report.restarted = true; - } else { - crate::bootstrap::set_backend_kill_intended(false); - } - Ok(report) } #[cfg(test)] mod tests { use super::*; + #[test] + fn aborted_reset_future_does_not_cancel_retained_finalization() { + let entered = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + let release = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + let finalized = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + let entered2 = entered.clone(); + let release2 = release.clone(); + let finalized2 = finalized.clone(); + + let task = tauri::async_runtime::spawn(run_retained_reset(move || { + entered2.store(true, std::sync::atomic::Ordering::SeqCst); + while !release2.load(std::sync::atomic::Ordering::SeqCst) { + std::thread::sleep(std::time::Duration::from_millis(5)); + } + finalized2.store(true, std::sync::atomic::Ordering::SeqCst); + Ok(()) + })); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(2); + while !entered.load(std::sync::atomic::Ordering::SeqCst) + && std::time::Instant::now() < deadline + { + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert!(entered.load(std::sync::atomic::Ordering::SeqCst)); + task.abort(); + release.store(true, std::sync::atomic::Ordering::SeqCst); + + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(2); + while !finalized.load(std::sync::atomic::Ordering::SeqCst) + && std::time::Instant::now() < deadline + { + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert!( + finalized.load(std::sync::atomic::Ordering::SeqCst), + "dropping the IPC await must not cancel backend restart/finalization" + ); + } + fn roots(tmp: &Path) -> Roots { Roots { data: tmp.join("OmniVoice"), diff --git a/frontend/src-tauri/src/setup.rs b/frontend/src-tauri/src/setup.rs index 32c0c09cb..835ba5c9b 100644 --- a/frontend/src-tauri/src/setup.rs +++ b/frontend/src-tauri/src/setup.rs @@ -230,6 +230,59 @@ fn write_portable_pointer(base: &Path) -> Option<&'static str> { Some(flavour) } +#[derive(Debug)] +struct FileSnapshot { + path: PathBuf, + contents: Option>, +} + +fn snapshot_file(path: PathBuf) -> Result { + let contents = match fs::read(&path) { + Ok(contents) => Some(contents), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => None, + Err(error) => return Err(format!("Could not snapshot {}: {error}", path.display())), + }; + Ok(FileSnapshot { path, contents }) +} + +fn snapshot_portable_location() -> Result, String> { + [portable_pointer_path(), config::config_path_for_machine()] + .into_iter() + .flatten() + .map(snapshot_file) + .collect() +} + +fn restore_file(snapshot: FileSnapshot) -> Result<(), String> { + match snapshot.contents { + Some(contents) => { + if let Some(parent) = snapshot.path.parent() { + fs::create_dir_all(parent) + .map_err(|error| format!("Could not restore {}: {error}", parent.display()))?; + } + fs::write(&snapshot.path, contents) + .map_err(|error| format!("Could not restore {}: {error}", snapshot.path.display())) + } + None => match fs::remove_file(&snapshot.path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(format!("Could not restore {}: {error}", snapshot.path.display())), + }, + } +} + +fn restore_portable_location(snapshots: Vec) -> Result<(), String> { + let errors: Vec = snapshots + .into_iter() + .filter_map(|snapshot| restore_file(snapshot).err()) + .collect(); + if errors.is_empty() { + Ok(()) + } else { + Err(errors.join("; ")) + } +} + /// Persist where a relocated portable folder lives, so the next launch finds /// it. Prefers the pointer file (portability preserved); falls back to the /// per-user config when the app folder is read-only. @@ -882,10 +935,27 @@ pub fn check_install_target(path: String) -> TargetCheck { /// parked) bootstrap. Any `Err` keeps the app in `AwaitingSetup` with the /// message surfaced on the setup screen — nothing was installed. #[tauri::command] -pub fn complete_setup( +pub async fn complete_setup( app: tauri::AppHandle, state: tauri::State<'_, BootstrapState>, plan: InstallPlan, +) -> Result<(), String> { + let owned_state = BootstrapState { + stage: state.stage.clone(), + logs: state.logs.clone(), + }; + tauri::async_runtime::spawn_blocking(move || complete_setup_blocking(app, owned_state, plan)) + .await + .map_err(|error| { + log::error!("Setup task failed to join: {error}"); + "setup_task_failed".to_string() + })? +} + +fn complete_setup_blocking( + app: tauri::AppHandle, + state: BootstrapState, + plan: InstallPlan, ) -> Result<(), String> { if !matches!(plan.install_mode.as_str(), "installed" | "portable") { return Err(format!("Unknown install mode: {}", plan.install_mode)); @@ -942,34 +1012,12 @@ pub fn complete_setup( } cfg.locale = plan.locale.clone().filter(|l| !l.is_empty()); - if plan.install_mode == "portable" { - // Create the portable folder and seed config.json INSIDE it first, so - // `config_path` resolves portable from here on and the whole install - // (env + data + config) travels as one folder. - let base = planned_portable_base(&plan).ok_or("Portable anchor disappeared")?; - fs::create_dir_all(&base).map_err(|e| format!("Could not create {}: {e}", base.display()))?; - // Record WHERE it is before seeding it — `portable_base()` has to - // resolve to this folder on the next launch, and the config inside it - // cannot say so (nothing would know where to look). - if base != portable_anchor().map(|a| a.join(PORTABLE_DIR_NAME)).unwrap_or_default() { - let how = record_portable_dir(&app, &base)?; - log::info!("Portable folder relocated — recorded via {how}"); - } else { - // Back to the default: drop any earlier relocation so a stale - // pointer can't outrank it. - clear_portable_dir(&app); - } - config::save_config_at(&base.join("config.json"), &cfg)?; - } else { + if plan.install_mode != "portable" { for (dir, _) in &targets { - fs::create_dir_all(dir).map_err(|e| format!("Could not create {}: {e}", dir.display()))?; + fs::create_dir_all(dir) + .map_err(|e| format!("Could not create {}: {e}", dir.display()))?; } } - // The plan must actually persist before bootstrap starts — a swallowed - // write error here would bootstrap into a stale layout from disk while - // the UI reports success. - let cfg_path = config::config_path(&app).ok_or("Could not resolve the config file path")?; - config::save_config_at(&cfg_path, &cfg)?; // Custom paths are home-relative PII — log default-vs-custom flags, not // the raw locations. @@ -982,22 +1030,80 @@ pub fn complete_setup( custom(&cfg.models_dir), ); - // `--setup` re-entry: a backend from the previous configuration may - // still be serving. retry_bootstrap would attach to it and the new - // env/mirror/layout settings would never apply — tear it down so the - // restart spawns with the just-saved plan. (No-op on a true first run: - // nothing is listening yet.) - if crate::backend::port_in_use(crate::backend_port()) { - log::info!( - "Backend still running on port {} — restarting it so the new setup applies", - crate::backend_port() - ); - crate::backend::kill_orphan_on_port(crate::backend_port()); - std::thread::sleep(std::time::Duration::from_millis(500)); + // `--setup` re-entry: stop the previous backend before committing the new + // durable layout. A failed stop must leave the old config/pointer active; + // otherwise its automatic recovery would launch against a half-applied + // setup plan. + let persisted = crate::bootstrap::with_backend_stopped(&app, || -> Result<(), String> { + let previous_location = if plan.install_mode == "portable" { + Some(snapshot_portable_location()?) + } else { + None + }; + let result = (|| -> Result<(), String> { + if plan.install_mode == "portable" { + // Create the portable folder and seed config.json INSIDE it first, + // so `config_path` resolves portable from here on and the whole + // install (env + data + config) travels as one folder. + let base = planned_portable_base(&plan).ok_or("Portable anchor disappeared")?; + fs::create_dir_all(&base) + .map_err(|e| format!("Could not create {}: {e}", base.display()))?; + // Record WHERE it is before seeding it — `portable_base()` has to + // resolve to this folder on the next launch, and the config inside + // it cannot say so (nothing would know where to look). + if base + != portable_anchor() + .map(|a| a.join(PORTABLE_DIR_NAME)) + .unwrap_or_default() + { + let how = record_portable_dir(&app, &base)?; + log::info!("Portable folder relocated — recorded via {how}"); + } else { + // Back to the default: drop any earlier relocation so a stale + // pointer can't outrank it. + clear_portable_dir(&app); + } + config::save_config_at(&base.join("config.json"), &cfg)?; + } + // The plan must actually persist before bootstrap starts — a swallowed + // write error here would bootstrap into a stale layout from disk while + // the UI reports success. + let cfg_path = + config::config_path(&app).ok_or("Could not resolve the config file path")?; + config::save_config_at(&cfg_path, &cfg) + })(); + match (result, previous_location) { + (Err(error), Some(snapshot)) => match restore_portable_location(snapshot) { + Ok(()) => Err(error), + Err(rollback_error) => Err(format!( + "{error}; could not restore the previous portable location: {rollback_error}" + )), + }, + (result, _) => result, + } + }); + match persisted { + Err(error) => { + // with_backend_stopped already re-arms the old backend for every + // non-terminal caller. + log::warn!("Setup could not stop the previous backend: {error}"); + return Err("backend_stop_failed".into()); + } + Ok(Err(error)) => { + // The stop succeeded but persistence did not; restore service on + // the surviving layout instead of leaving the settings UI down. + crate::bootstrap::respawn_backend( + app, + state.stage.clone(), + state.logs.clone(), + ); + return Err(error); + } + Ok(Ok(())) => {} } set_stage(&state.stage, BootstrapStage::Checking); - crate::bootstrap::retry_bootstrap(app, state); + crate::bootstrap::respawn_backend(app, state.stage, state.logs); Ok(()) } @@ -1036,6 +1142,29 @@ mod tests { assert_eq!(flavour, "pointer-absolute"); } + #[test] + fn file_snapshot_restores_present_and_absent_files() { + let root = std::env::temp_dir().join(format!( + "ov-portable-snapshot-{}-{:?}", + std::process::id(), + std::thread::current().id() + )); + fs::create_dir_all(&root).unwrap(); + let present = root.join("present"); + let absent = root.join("absent"); + fs::write(&present, b"old").unwrap(); + let present_snapshot = snapshot_file(present.clone()).unwrap(); + let absent_snapshot = snapshot_file(absent.clone()).unwrap(); + + fs::write(&present, b"new").unwrap(); + fs::write(&absent, b"new").unwrap(); + restore_portable_location(vec![present_snapshot, absent_snapshot]).unwrap(); + + assert_eq!(fs::read(&present).unwrap(), b"old"); + assert!(!absent.exists()); + fs::remove_dir_all(root).unwrap(); + } + /// Portable-folder relocation (#1403 follow-up). One test fn, not several: /// it mutates `APPIMAGE`, which is process-global, and Rust tests run in /// parallel — same rationale as the uv-env test below. diff --git a/frontend/src-tauri/src/tools.rs b/frontend/src-tauri/src/tools.rs index 4ebab9ef3..a5f1ab3a4 100644 --- a/frontend/src-tauri/src/tools.rs +++ b/frontend/src-tauri/src/tools.rs @@ -3,10 +3,17 @@ use std::fs; use std::io; use std::path::{Path, PathBuf}; -use std::process::{Command, Output, Stdio}; +use std::process::{Child, Command, ExitStatus, Output, Stdio}; +#[cfg(target_os = "macos")] +use std::sync::atomic::{AtomicU8, Ordering}; use std::sync::{Arc, Mutex}; use std::time::Duration; +#[cfg(unix)] +use std::os::fd::{AsRawFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; +#[cfg(unix)] +use std::os::unix::net::UnixStream; + use crate::config::get_effective_region; #[allow(unused_imports)] // Used in cfg(linux) and cfg(windows) blocks use crate::config::resolve_github_url; @@ -23,12 +30,11 @@ use crate::bootstrap::{BootstrapStage, set_stage}; /// (0x08000000) runs the child with no console at all; every caller already /// pipes/among nulls stdout+stderr, so nothing visible or logged is lost. /// -/// This is the single chokepoint every bootstrap/tools spawn routes through, -/// mirroring the flag the backend spawn (`backend.rs`) and the `nvidia-smi` -/// probe (`setup.rs`) already set inline. No-op on macOS/Linux — there is no -/// per-process console to hide there, so behaviour is unchanged on those -/// platforms (default-parity rule: the *visible* default is now identical — -/// no stray windows — across all three). +/// Short-lived bootstrap/tools spawns route through this chokepoint; +/// long-running children use [`spawn_process_tree`], which preserves the same +/// flag while also making descendants terminable. No-op on macOS/Linux — +/// there is no per-process console to hide there, so behaviour is unchanged on +/// those platforms (default-parity rule: no stray windows on any OS). /// /// Returns the same `&mut Command` so it chains inline: /// `no_window(Command::new(p).args([..])).output()`. @@ -43,6 +49,583 @@ pub fn no_window(cmd: &mut Command) -> &mut Command { cmd } +/// A spawned child plus its non-reusable OS containment primitive. Unix uses +/// an inherited process group plus a CLOEXEC drain channel for deliberately +/// nested operation groups; Windows uses a Job Object assigned while the root +/// is suspended, before it can create any descendants. +pub struct ContainedChild { + pub child: Child, + pub tree: OwnedProcessTree, +} + +pub struct OwnedProcessTree { + root_pid: u32, + terminated: bool, + #[cfg(unix)] + process_group: libc::pid_t, + #[cfg(unix)] + nested_drain: OwnedFd, + #[cfg(target_os = "macos")] + exit_kqueue: OwnedFd, + #[cfg(target_os = "macos")] + root_exit_state: AtomicU8, + #[cfg(windows)] + job: std::os::windows::io::OwnedHandle, +} + +impl OwnedProcessTree { + #[cfg(unix)] + fn root_exited_unreaped(&self) -> io::Result { + #[cfg(target_os = "macos")] + { + // Darwin may reject waitid(WNOWAIT) with EPERM. The kqueue was + // registered while this Child was created, so NOTE_EXIT observes + // the same process identity without consuming its wait status. + // NOTE_REAP keeps the filter alive through wait(2), allowing us + // to reject a group ID after another caller consumed that status. + const EXITED: u8 = 1; + const REAPED: u8 = 2; + if self.root_exit_state.load(Ordering::Acquire) == REAPED { + return Err(io::Error::from_raw_os_error(libc::ECHILD)); + } + let mut event: libc::kevent = unsafe { std::mem::zeroed() }; + let timeout = libc::timespec { + tv_sec: 0, + tv_nsec: 0, + }; + let result = unsafe { + libc::kevent( + self.exit_kqueue.as_raw_fd(), + std::ptr::null(), + 0, + &mut event, + 1, + &timeout, + ) + }; + if result < 0 { + return Err(io::Error::last_os_error()); + } + if result == 0 { + return Ok(self.root_exit_state.load(Ordering::Acquire) == EXITED); + } + if event.flags & libc::EV_ERROR != 0 { + return Err(io::Error::from_raw_os_error(event.data as i32)); + } + if event.fflags & libc::NOTE_REAP != 0 { + self.root_exit_state.store(REAPED, Ordering::Release); + return Err(io::Error::from_raw_os_error(libc::ECHILD)); + } + if event.fflags & libc::NOTE_EXIT != 0 { + self.root_exit_state.store(EXITED, Ordering::Release); + } + return Ok(self.root_exit_state.load(Ordering::Acquire) == EXITED); + } + #[cfg(not(target_os = "macos"))] + { + let mut info: libc::siginfo_t = unsafe { std::mem::zeroed() }; + let result = unsafe { + libc::waitid( + libc::P_PID, + self.root_pid as libc::id_t, + &mut info, + libc::WEXITED | libc::WNOHANG | libc::WNOWAIT, + ) + }; + if result != 0 { + return Err(io::Error::last_os_error()); + } + Ok(unsafe { info.si_pid() } != 0) + } + } + + #[cfg(unix)] + fn signal_group(&self, signal: libc::c_int) -> io::Result<()> { + if unsafe { libc::kill(-self.process_group, signal) } == 0 { + return Ok(()); + } + let error = io::Error::last_os_error(); + if error.raw_os_error() == Some(libc::ESRCH) { + Ok(()) + } else { + Err(error) + } + } + + fn force_terminate(&mut self) -> io::Result<()> { + if self.terminated { + return Ok(()); + } + #[cfg(unix)] + self.signal_group(libc::SIGKILL)?; + #[cfg(windows)] + { + use std::os::windows::io::AsRawHandle; + use windows::Win32::Foundation::HANDLE; + use windows::Win32::System::JobObjects::TerminateJobObject; + + unsafe { + TerminateJobObject(HANDLE(self.job.as_raw_handle()), 1) + .map_err(windows_error)?; + } + } + self.terminated = true; + Ok(()) + } + + #[cfg(unix)] + fn force_terminate_after_root_exit(&mut self) -> io::Result<()> { + if self.terminated { + return Ok(()); + } + match self.signal_group(libc::SIGKILL) { + Ok(()) => {} + #[cfg(target_os = "macos")] + Err(error) if error.raw_os_error() == Some(libc::EPERM) => { + // XNU excludes zombies when iterating an explicit process + // group, then reports EPERM when it found no signalable live + // member. The unreaped root still reserves this exact group; + // the nested-drain join that follows catches any descendant + // which actually survived the signal attempt. + } + Err(error) => return Err(error), + } + self.terminated = true; + Ok(()) + } + + fn wait_nested_drain(&mut self, timeout: Duration) -> io::Result<()> { + #[cfg(unix)] + { + let fd = self.nested_drain.as_raw_fd(); + let deadline = std::time::Instant::now() + timeout; + loop { + if nested_drain_eof(fd)? { + return Ok(()); + } + let remaining = deadline.saturating_duration_since(std::time::Instant::now()); + if remaining.is_zero() { + return Err(io::Error::new( + io::ErrorKind::TimedOut, + format!( + "nested backend operations did not drain within {timeout:?}" + ), + )); + } + let millis = remaining.as_millis().clamp(1, i32::MAX as u128) as i32; + let mut pollfd = libc::pollfd { + fd, + events: libc::POLLIN | libc::POLLHUP | libc::POLLERR, + revents: 0, + }; + let result = unsafe { libc::poll(&mut pollfd, 1, millis) }; + if result > 0 { + continue; + } + if result == 0 { + return Err(io::Error::new( + io::ErrorKind::TimedOut, + format!( + "nested backend operations did not drain within {timeout:?}" + ), + )); + } + let error = io::Error::last_os_error(); + if error.kind() != io::ErrorKind::Interrupted { + return Err(error); + } + } + } + #[cfg(not(unix))] + { + let _ = timeout; + Ok(()) + } + } +} + +#[cfg(unix)] +fn nested_drain_eof(fd: RawFd) -> io::Result { + let mut buffer = [0u8; 64]; + loop { + let read = unsafe { libc::read(fd, buffer.as_mut_ptr().cast(), buffer.len()) }; + if read == 0 { + return Ok(true); + } + if read > 0 { + continue; + } + let error = io::Error::last_os_error(); + if error.kind() == io::ErrorKind::WouldBlock { + return Ok(false); + } + if error.kind() != io::ErrorKind::Interrupted { + return Err(error); + } + } +} + +#[cfg(unix)] +fn create_nested_drain() -> io::Result<(OwnedFd, OwnedFd)> { + // UnixStream::pair creates both descriptors CLOEXEC atomically inside the + // standard library on Linux and macOS. A pipe()+fcntl sequence would have + // an inheritance race with another thread spawning between those calls. + let (read, write) = UnixStream::pair()?; + read.set_nonblocking(true)?; + let read = unsafe { OwnedFd::from_raw_fd(read.into_raw_fd()) }; + let write = unsafe { OwnedFd::from_raw_fd(write.into_raw_fd()) }; + Ok((read, write)) +} + +#[cfg(target_os = "macos")] +fn watch_process_exit(pid: u32) -> io::Result { + let raw_fd = unsafe { libc::kqueue() }; + if raw_fd < 0 { + return Err(io::Error::last_os_error()); + } + let queue = unsafe { OwnedFd::from_raw_fd(raw_fd) }; + let change = libc::kevent { + ident: pid as libc::uintptr_t, + filter: libc::EVFILT_PROC, + flags: libc::EV_ADD | libc::EV_ENABLE, + fflags: libc::NOTE_EXIT | libc::NOTE_REAP, + data: 0, + udata: std::ptr::null_mut(), + }; + let result = unsafe { + libc::kevent( + queue.as_raw_fd(), + &change, + 1, + std::ptr::null_mut(), + 0, + std::ptr::null(), + ) + }; + if result < 0 { + return Err(io::Error::last_os_error()); + } + Ok(queue) +} + +fn nested_drain_timeout() -> Duration { + #[cfg(debug_assertions)] + if let Some(timeout) = std::env::var("OMNIVOICE_TEST_NESTED_DRAIN_TIMEOUT_MS") + .ok() + .and_then(|value| value.parse::().ok()) + .filter(|&millis| millis > 0) + { + return Duration::from_millis(timeout); + } + Duration::from_secs(5) +} + +impl Drop for OwnedProcessTree { + fn drop(&mut self) { + #[cfg(unix)] + if !self.terminated && self.root_exited_unreaped().is_ok() { + // Cancellation/panic fallback. The waitid success proves the root + // is still our unreaped child, so its group ID cannot have been + // reused. ECHILD deliberately falls through without signalling. + let _ = self.signal_group(libc::SIGKILL); + } + // Windows JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE provides this fallback + // automatically when `job` is dropped. + } +} + +/// Spawn into deterministic containment. No descendant discovery is involved: +/// ordinary children inherit this process group/job at creation, while nested +/// Python operation owners inherit a drain writer which Rust joins on teardown. +pub fn spawn_process_tree(cmd: &mut Command) -> io::Result { + // Python operation owners may open a nested session for local timeouts; + // their inherited drain writer makes that handoff joinable from Rust. + cmd.env("OMNIVOICE_DESKTOP_CONTAINED", "1"); + #[cfg(unix)] + { + use std::os::unix::process::CommandExt; + let (nested_drain, nested_drain_write) = create_nested_drain()?; + let nested_drain_fd = nested_drain_write.as_raw_fd(); + cmd.env("OMNIVOICE_DESKTOP_DRAIN_FD", nested_drain_fd.to_string()); + unsafe { + cmd.pre_exec(move || { + let flags = libc::fcntl(nested_drain_fd, libc::F_GETFD); + if flags < 0 + || libc::fcntl( + nested_drain_fd, + libc::F_SETFD, + flags & !libc::FD_CLOEXEC, + ) < 0 + { + return Err(io::Error::last_os_error()); + } + Ok(()) + }); + } + cmd.process_group(0); + #[cfg(target_os = "macos")] + let mut child = cmd.spawn()?; + #[cfg(not(target_os = "macos"))] + let child = cmd.spawn()?; + drop(nested_drain_write); + let root_pid = child.id(); + let process_group = i32::try_from(root_pid) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "pid exceeds i32"))?; + #[cfg(target_os = "macos")] + let exit_kqueue = match watch_process_exit(root_pid) { + Ok(queue) => queue, + Err(error) => { + unsafe { + libc::kill(-process_group, libc::SIGKILL); + } + let _ = child.wait(); + return Err(error); + } + }; + return Ok(ContainedChild { + child, + tree: OwnedProcessTree { + root_pid, + terminated: false, + process_group, + nested_drain, + #[cfg(target_os = "macos")] + exit_kqueue, + #[cfg(target_os = "macos")] + root_exit_state: AtomicU8::new(0), + }, + }); + } + #[cfg(windows)] + { + use std::os::windows::process::CommandExt; + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + const CREATE_NEW_PROCESS_GROUP: u32 = 0x0000_0200; + const CREATE_SUSPENDED: u32 = 0x0000_0004; + + let job = create_kill_on_close_job()?; + cmd.creation_flags(CREATE_NO_WINDOW | CREATE_NEW_PROCESS_GROUP | CREATE_SUSPENDED); + let mut child = cmd.spawn()?; + if let Err(error) = assign_job_and_resume(&job, &child) { + let _ = child.kill(); + let _ = child.wait(); + return Err(error); + } + let root_pid = child.id(); + return Ok(ContainedChild { + child, + tree: OwnedProcessTree { + root_pid, + terminated: false, + job, + }, + }); + } + #[cfg(not(any(unix, windows)))] + { + let _ = cmd; + Err(io::Error::new( + io::ErrorKind::Unsupported, + "process containment is unsupported on this platform", + )) + } +} + +/// Observe an unexpected root death without a PID check/signal race. Unix +/// leaves the root unreaped while signalling its still-reserved process group +/// and waiting for every nested owner to drain; Windows terminates the stable +/// Job handle after the stable Child handle reports exit. +pub fn contained_child_exit( + child: &mut Child, + tree: &mut OwnedProcessTree, +) -> io::Result> { + #[cfg(unix)] + { + match tree.root_exited_unreaped() { + Ok(false) => return Ok(None), + Ok(true) => { + // Do not reap the root until group cleanup succeeds: the + // zombie is what keeps this process-group ID non-reusable. + tree.force_terminate_after_root_exit()?; + tree.wait_nested_drain(nested_drain_timeout())?; + let status = child.try_wait()?; + return Ok(status); + } + Err(error) if error.raw_os_error() == Some(libc::ECHILD) => { + // Another owner already reaped this root. Refuse to signal its + // numeric group: it may since have been reused by a foreign + // process. Production lifecycle paths never take this branch. + return Err(io::Error::new( + io::ErrorKind::Other, + format!( + "process-tree root {} was already reaped; refusing a reusable group ID", + tree.root_pid + ), + )); + } + Err(error) => return Err(error), + } + } + #[cfg(windows)] + { + let status = child.try_wait()?; + if status.is_some() { + tree.force_terminate()?; + } + Ok(status) + } + #[cfg(not(any(unix, windows)))] + { + let _ = tree; + child.try_wait() + } +} + +/// Ask the contained tree to stop, then force the same stable containment +/// primitive before reaping the root. On Unix the unreaped root reserves the +/// process-group ID and keeps drain-timeout retries safe; a foreign group can +/// never be substituted between an identity check and signal. Windows never +/// signals by PID at all. +pub fn terminate_process_tree( + child: &mut Child, + tree: &mut OwnedProcessTree, + graceful_timeout: Duration, +) -> io::Result { + let pid = child.id(); + #[cfg(unix)] + match tree.root_exited_unreaped() { + Ok(true) => { + tree.force_terminate_after_root_exit()?; + tree.wait_nested_drain(nested_drain_timeout())?; + return child.wait(); + } + Ok(false) => tree.signal_group(libc::SIGTERM)?, + Err(error) if error.raw_os_error() == Some(libc::ECHILD) => { + return Err(io::Error::new( + io::ErrorKind::Other, + format!( + "process-tree root {pid} was already reaped; refusing a reusable group ID" + ), + )); + } + Err(error) => return Err(error), + } + #[cfg(windows)] + { + // A console-less GUI child has no reliable graceful control event. + // Terminating the stable job is the existing forced fallback, now + // guaranteed to include every descendant. + tree.force_terminate()?; + } + + let deadline = std::time::Instant::now() + graceful_timeout; + while std::time::Instant::now() < deadline { + #[cfg(unix)] + if tree.root_exited_unreaped()? { + tree.force_terminate_after_root_exit()?; + tree.wait_nested_drain(nested_drain_timeout())?; + return child.wait(); + } + #[cfg(windows)] + if let Some(status) = child.try_wait()? { + return Ok(status); + } + std::thread::sleep(Duration::from_millis(50)); + } + + log::warn!( + "Process tree rooted at pid {pid} did not stop within {graceful_timeout:?}; forcing it" + ); + tree.force_terminate()?; + let _ = child.kill(); + tree.wait_nested_drain(nested_drain_timeout())?; + child.wait() +} + +#[cfg(windows)] +fn windows_error(error: windows_core::Error) -> io::Error { + io::Error::new(io::ErrorKind::Other, error.to_string()) +} + +#[cfg(windows)] +fn create_kill_on_close_job() -> io::Result { + use std::os::windows::io::{AsRawHandle, FromRawHandle}; + use windows::Win32::Foundation::HANDLE; + use windows::Win32::System::JobObjects::{ + CreateJobObjectW, JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, + JOBOBJECT_EXTENDED_LIMIT_INFORMATION, JobObjectExtendedLimitInformation, + SetInformationJobObject, + }; + + let job = unsafe { CreateJobObjectW(None, None).map_err(windows_error)? }; + let job = unsafe { std::os::windows::io::OwnedHandle::from_raw_handle(job.0) }; + let mut info = JOBOBJECT_EXTENDED_LIMIT_INFORMATION::default(); + info.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + unsafe { + SetInformationJobObject( + HANDLE(job.as_raw_handle()), + JobObjectExtendedLimitInformation, + std::ptr::addr_of!(info).cast(), + std::mem::size_of_val(&info) as u32, + ) + .map_err(windows_error)?; + Ok(job) + } +} + +#[cfg(windows)] +fn assign_job_and_resume( + job: &std::os::windows::io::OwnedHandle, + child: &Child, +) -> io::Result<()> { + use std::os::windows::io::{AsRawHandle, FromRawHandle}; + use windows::Win32::Foundation::HANDLE; + use windows::Win32::System::Diagnostics::ToolHelp::{ + CreateToolhelp32Snapshot, TH32CS_SNAPTHREAD, THREADENTRY32, Thread32First, Thread32Next, + }; + use windows::Win32::System::JobObjects::AssignProcessToJobObject; + use windows::Win32::System::Threading::{OpenThread, ResumeThread, THREAD_SUSPEND_RESUME}; + + unsafe { + AssignProcessToJobObject( + HANDLE(job.as_raw_handle()), + HANDLE(child.as_raw_handle()), + ) + .map_err(windows_error)?; + } + + let snapshot = unsafe { + CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0).map_err(windows_error)? + }; + let snapshot = unsafe { std::os::windows::io::OwnedHandle::from_raw_handle(snapshot.0) }; + let mut entry = THREADENTRY32 { + dwSize: std::mem::size_of::() as u32, + ..Default::default() + }; + unsafe { + Thread32First(HANDLE(snapshot.as_raw_handle()), &mut entry).map_err(windows_error)?; + } + loop { + if entry.th32OwnerProcessID == child.id() { + let thread = unsafe { + OpenThread(THREAD_SUSPEND_RESUME, false, entry.th32ThreadID) + .map_err(windows_error)? + }; + let thread = unsafe { std::os::windows::io::OwnedHandle::from_raw_handle(thread.0) }; + if unsafe { ResumeThread(HANDLE(thread.as_raw_handle())) } == u32::MAX { + return Err(io::Error::last_os_error()); + } + return Ok(()); + } + if unsafe { Thread32Next(HANDLE(snapshot.as_raw_handle()), &mut entry) }.is_err() { + return Err(io::Error::new( + io::ErrorKind::NotFound, + "suspended child primary thread was not found", + )); + } + } +} + // Version of the Astral `uv` binary we download at first run when no system // uv is on PATH. Pinned for reproducibility — bump alongside the uv.lock // when the toolchain needs a newer uv. @@ -624,6 +1207,62 @@ mod uv_tests { use super::*; use std::ffi::OsStr; + #[cfg(unix)] + #[test] + fn contained_exit_probe_preserves_a_live_child() { + let mut command = Command::new("sleep"); + command.arg("30"); + let ContainedChild { + mut child, + mut tree, + } = spawn_process_tree(&mut command).expect("spawn contained test child"); + + assert!(contained_child_exit(&mut child, &mut tree).unwrap().is_none()); + terminate_process_tree(&mut child, &mut tree, Duration::ZERO) + .expect("terminate the still-owned process tree"); + } + + #[cfg(unix)] + #[test] + fn unexpected_root_exit_without_descendants_is_reaped_cleanly() { + let mut command = Command::new("sh"); + command.args(["-c", "exit 7"]); + let ContainedChild { + mut child, + mut tree, + } = spawn_process_tree(&mut command).expect("spawn contained test child"); + let deadline = std::time::Instant::now() + Duration::from_secs(5); + + loop { + if let Some(status) = contained_child_exit(&mut child, &mut tree) + .expect("clean an already-exited root") + { + assert_eq!(status.code(), Some(7)); + break; + } + assert!(std::time::Instant::now() < deadline, "child never exited"); + std::thread::sleep(Duration::from_millis(10)); + } + } + + #[cfg(unix)] + #[test] + fn reaped_root_refuses_to_signal_a_reusable_process_group() { + let mut command = Command::new("sleep"); + command.arg("30"); + let ContainedChild { + mut child, + mut tree, + } = spawn_process_tree(&mut command).expect("spawn contained test child"); + child.kill().unwrap(); + child.wait().unwrap(); // deliberately discard the stable root identity + + let error = terminate_process_tree(&mut child, &mut tree, Duration::ZERO) + .expect_err("a reusable numeric process-group id must never be signalled"); + assert!(error.to_string().contains("already reaped")); + assert!(error.to_string().contains("refusing")); + } + #[test] fn installer_uses_app_private_unmanaged_mode() { let mut command = Command::new("installer"); diff --git a/frontend/src-tauri/src/uninstall.rs b/frontend/src-tauri/src/uninstall.rs index 09201f2a8..51a597d06 100644 --- a/frontend/src-tauri/src/uninstall.rs +++ b/frontend/src-tauri/src/uninstall.rs @@ -21,8 +21,9 @@ use std::fs; use std::path::{Path, PathBuf}; use serde::Serialize; +use tauri::Manager; -use crate::{backend_port, AppFlags}; +use crate::AppFlags; #[derive(Serialize, Clone, Debug)] pub struct UninstallTarget { @@ -108,12 +109,13 @@ fn target(key: &str, path: PathBuf, shared: bool) -> UninstallTarget { /// Every folder this install owns, with sizes — what the confirmation UI shows. /// Honors custom + portable locations via the shared resolvers. -#[tauri::command] -pub fn uninstall_scan(app: tauri::AppHandle) -> Vec { - let data = crate::setup::resolved_data_dir(&app).unwrap_or_else(crate::setup::default_data_dir); - let env = crate::setup::env_root(&app); +fn uninstall_scan_blocking( + app: &tauri::AppHandle, +) -> Vec { + let data = crate::setup::resolved_data_dir(app).unwrap_or_else(crate::setup::default_data_dir); + let env = crate::setup::env_root(app); let models = - crate::setup::resolved_models_dir(&app).unwrap_or_else(crate::setup::default_models_dir); + crate::setup::resolved_models_dir(app).unwrap_or_else(crate::setup::default_models_dir); let mut out = vec![ // Voices, projects, DB, generated audio, the backend's rolling log. @@ -139,6 +141,19 @@ pub fn uninstall_scan(app: tauri::AppHandle) -> Vec { out } +async fn run_scan_off_thread( + scan: impl FnOnce() -> T + Send + 'static, +) -> Result { + tauri::async_runtime::spawn_blocking(scan) + .await + .map_err(|error| format!("uninstall_scan_failed: {error}")) +} + +#[tauri::command] +pub async fn uninstall_scan(app: tauri::AppHandle) -> Result, String> { + run_scan_off_thread(move || uninstall_scan_blocking(&app)).await +} + /// `~/.config/omnivoice` — the directory holding the durable per-user env file. /// Mirrors `backend/core/user_env.py::USER_ENV_PATH`, which uses `expanduser` /// on every platform, so this is `%USERPROFILE%\.config\omnivoice` on Windows. @@ -146,26 +161,7 @@ fn user_env_dir() -> Option { dirs_next::home_dir().map(|h| h.join(".config").join("omnivoice")) } -/// Stop the backend and delete the scanned folders. `include_models` opts into -/// the shared Hugging Face cache. Returns what was removed; the caller quits the -/// app afterwards (the Python env it runs on is gone, so there is nothing to -/// return to). -#[tauri::command] -pub fn uninstall_purge( - app: tauri::AppHandle, - include_models: bool, - flags: tauri::State<'_, AppFlags>, -) -> Result { - // Mark the app as quitting BEFORE the backend dies, so the #567 supervisor - // treats the death as intentional and doesn't respawn a backend into the - // very directories we are about to delete. - flags - .quitting - .store(true, std::sync::atomic::Ordering::SeqCst); - crate::bootstrap::set_backend_kill_intended(true); - crate::backend::kill_orphan_on_port(backend_port()); - std::thread::sleep(std::time::Duration::from_millis(600)); - +fn purge_targets(targets: Vec, include_models: bool) -> UninstallReport { let home = dirs_next::home_dir(); let mut report = UninstallReport { removed: vec![], @@ -173,7 +169,7 @@ pub fn uninstall_purge( freed_bytes: 0, }; - for t in uninstall_scan(app.clone()) { + for t in targets { if !t.exists { continue; } @@ -198,13 +194,429 @@ pub fn uninstall_purge( } } } - Ok(report) + report +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +struct UninstallClaim(u64); + +static NEXT_UNINSTALL_CLAIM: std::sync::atomic::AtomicU64 = + std::sync::atomic::AtomicU64::new(1); + +fn claim_uninstall( + uninstalling: &std::sync::atomic::AtomicBool, + owner: &std::sync::atomic::AtomicU64, +) -> Result { + uninstalling + .compare_exchange( + false, + true, + std::sync::atomic::Ordering::SeqCst, + std::sync::atomic::Ordering::SeqCst, + ) + .map_err(|_| "uninstall_in_progress".to_string())?; + let claim = UninstallClaim(NEXT_UNINSTALL_CLAIM.fetch_add( + 1, + std::sync::atomic::Ordering::SeqCst, + )); + owner.store(claim.0, std::sync::atomic::Ordering::SeqCst); + Ok(claim) +} + +fn release_uninstall_claim( + uninstalling: &std::sync::atomic::AtomicBool, + owner: &std::sync::atomic::AtomicU64, + claim: UninstallClaim, +) -> bool { + if owner + .compare_exchange( + claim.0, + 0, + std::sync::atomic::Ordering::SeqCst, + std::sync::atomic::Ordering::SeqCst, + ) + .is_err() + { + return false; + } + let _ = uninstalling.compare_exchange( + true, + false, + std::sync::atomic::Ordering::SeqCst, + std::sync::atomic::Ordering::SeqCst, + ); + true +} + +fn catch_uninstall_panic( + operation: impl FnOnce() -> Result, +) -> Result { + std::panic::catch_unwind(std::panic::AssertUnwindSafe(operation)) + .unwrap_or_else(|_| Err("uninstall_task_panicked".to_string())) +} + +async fn run_retained_uninstall_task( + worker: impl FnOnce() -> T + Send + 'static, +) -> Result { + tauri::async_runtime::spawn_blocking(worker) + .await + .map_err(|error| format!("uninstall_task_failed: {error}")) +} + +/// Lifecycle-aware deletion core shared by the command and the real-child +/// regression harness. +#[doc(hidden)] +pub fn purge_uninstall_targets( + app: &tauri::AppHandle, + targets: Vec, + include_models: bool, +) -> Result { + crate::bootstrap::with_backend_stopped(app, || { + // Let Windows release any final executable/DLL handles before the + // managed environment is removed. Lifecycle ownership stays held, so + // no launch path can recreate the child during this grace period. + std::thread::sleep(std::time::Duration::from_millis(600)); + purge_targets(targets, include_models) + }) +} + +fn finish_uninstall_attempt( + uninstalling: &std::sync::atomic::AtomicBool, + owner: &std::sync::atomic::AtomicU64, + claim: UninstallClaim, + quitting: &std::sync::atomic::AtomicBool, + result: Result, + recover_backend: impl FnOnce(), + exit_app: impl FnOnce(), +) -> Result { + // Every completed purge is immediately followed by application exit. An + // Ok report may include per-target failures after other targets were + // already removed; restoring supervision then could respawn the backend + // into a partially deleted environment. Only an Err means teardown could + // not complete and returns control to a usable settings UI. + if result.is_ok() { + // The webview which invoked this command may have closed or reloaded + // while deletion ran. Exit from the retained native task instead of + // relying on a frontend follow-up that may never arrive. + quitting.store(true, std::sync::atomic::Ordering::SeqCst); + exit_app(); + } else { + let released = release_uninstall_claim(uninstalling, owner, claim); + // A real ExitRequested owns `quitting`; never clear it or resurrect a + // backend while that terminal teardown is waiting on lifecycle. + if released && !quitting.load(std::sync::atomic::Ordering::SeqCst) { + recover_backend(); + } + } + result +} + +fn finish_uninstall_join( + uninstalling: &std::sync::atomic::AtomicBool, + owner: &std::sync::atomic::AtomicU64, + claim: UninstallClaim, + quitting: &std::sync::atomic::AtomicBool, + joined: Result, E>, + recover_backend: impl FnOnce(), + exit_app: impl FnOnce(), +) -> Result { + match joined { + // The blocking task already finalized both its success and ordinary + // error paths, including when the invoking webview dropped its future. + Ok(result) => result, + Err(_) => finish_uninstall_attempt( + uninstalling, + owner, + claim, + quitting, + Err("uninstall_task_failed".to_string()), + recover_backend, + exit_app, + ), + } +} + +/// Stop the backend and delete the scanned folders. `include_models` opts into +/// the shared Hugging Face cache. The retained native task exits the app after +/// success (the Python env it runs on is gone, so there is nothing to return +/// to), even if the invoking webview has closed or reloaded. +#[tauri::command] +pub async fn uninstall_purge( + app: tauri::AppHandle, + include_models: bool, +) -> Result { + // Suppress backend launches without claiming terminal app exit. A normal + // CloseRequested must still preserve the main window while this native + // task owns the destructive operation. + let claim = { + let flags = app.state::(); + claim_uninstall(&flags.uninstalling, &flags.uninstall_owner)? + }; + let purge_app = app.clone(); + let joined = run_retained_uninstall_task(move || { + let result = catch_uninstall_panic(|| { + let targets = uninstall_scan_blocking(&purge_app); + purge_uninstall_targets(&purge_app, targets, include_models) + }); + // Keep finalization in the blocking task: dropping the IPC future (for + // example during a navigation) must neither strand supervision after + // failure nor skip native exit after destructive success. + let state = purge_app.state::(); + let stage = state.stage.clone(); + let logs = state.logs.clone(); + let recover_app = purge_app.clone(); + let exit_app = purge_app.clone(); + let flags = purge_app.state::(); + finish_uninstall_attempt( + &flags.uninstalling, + &flags.uninstall_owner, + claim, + &flags.quitting, + result, + move || { + if crate::bootstrap::backend_stop_recovery_safe() { + crate::bootstrap::respawn_backend(recover_app, stage, logs); + } + }, + move || exit_app.exit(0), + ) + }) + .await; + if let Err(error) = &joined { + log::error!("Uninstall task failed to join: {error}"); + } + + // A scheduler cancellation or finalizer failure never returned an in-task + // result. Roll that path back while the command future is still alive. + let state = app.state::(); + let stage = state.stage.clone(); + let logs = state.logs.clone(); + let recover_app = app.clone(); + let exit_app = app.clone(); + let flags = app.state::(); + finish_uninstall_join( + &flags.uninstalling, + &flags.uninstall_owner, + claim, + &flags.quitting, + joined, + move || { + if crate::bootstrap::backend_stop_recovery_safe() { + crate::bootstrap::respawn_backend(recover_app, stage, logs); + } + }, + move || exit_app.exit(0), + ) } #[cfg(test)] mod tests { use super::*; + #[test] + fn failed_uninstall_restores_backend_supervision() { + let uninstalling = std::sync::atomic::AtomicBool::new(false); + let owner = std::sync::atomic::AtomicU64::new(0); + let quitting = std::sync::atomic::AtomicBool::new(false); + let recoveries = std::cell::Cell::new(0); + let exits = std::cell::Cell::new(0); + + let claim = claim_uninstall(&uninstalling, &owner).unwrap(); + let result: Result<(), String> = Err("backend tree is still running".into()); + assert!(finish_uninstall_attempt( + &uninstalling, + &owner, + claim, + &quitting, + result, + || { + assert!(!uninstalling.load(std::sync::atomic::Ordering::SeqCst)); + recoveries.set(recoveries.get() + 1); + }, + || exits.set(1), + ) + .is_err()); + assert!(!uninstalling.load(std::sync::atomic::Ordering::SeqCst)); + assert!(!quitting.load(std::sync::atomic::Ordering::SeqCst)); + assert_eq!(recoveries.get(), 1, "a failed stop must re-arm the backend"); + assert_eq!(exits.get(), 0); + + let claim = claim_uninstall(&uninstalling, &owner).unwrap(); + assert!(finish_uninstall_attempt( + &uninstalling, + &owner, + claim, + &quitting, + Ok(()), + || recoveries.set(recoveries.get() + 1), + || exits.set(1), + ) + .is_ok()); + assert!(quitting.load(std::sync::atomic::Ordering::SeqCst)); + assert!(uninstalling.load(std::sync::atomic::Ordering::SeqCst)); + assert_eq!( + recoveries.get(), + 1, + "a successful purge exits without restart" + ); + assert_eq!( + exits.get(), + 1, + "native success must not depend on the webview" + ); + + quitting.store(false, std::sync::atomic::Ordering::SeqCst); + let partial = UninstallReport { + removed: vec!["environment".into()], + failed: vec!["models".into()], + freed_bytes: 1, + }; + assert!(finish_uninstall_attempt( + &uninstalling, + &owner, + claim, + &quitting, + Ok(partial), + || recoveries.set(recoveries.get() + 1), + || exits.set(2), + ) + .is_ok()); + assert!( + quitting.load(std::sync::atomic::Ordering::SeqCst), + "a partial purge must still quit instead of respawning into deleted targets" + ); + assert_eq!(recoveries.get(), 1, "a partial purge must not restart"); + assert_eq!(exits.get(), 2); + } + + #[test] + fn join_failure_recovers_unless_a_real_exit_already_owns_shutdown() { + let uninstalling = std::sync::atomic::AtomicBool::new(false); + let owner = std::sync::atomic::AtomicU64::new(0); + let quitting = std::sync::atomic::AtomicBool::new(false); + let recoveries = std::cell::Cell::new(0); + + let claim = claim_uninstall(&uninstalling, &owner).unwrap(); + let joined: Result, ()> = Err(()); + assert!(finish_uninstall_join( + &uninstalling, + &owner, + claim, + &quitting, + joined, + || recoveries.set(recoveries.get() + 1), + || panic!("a failed task must not exit"), + ) + .is_err()); + assert!(!uninstalling.load(std::sync::atomic::Ordering::SeqCst)); + assert_eq!(recoveries.get(), 1); + + let claim = claim_uninstall(&uninstalling, &owner).unwrap(); + quitting.store(true, std::sync::atomic::Ordering::SeqCst); + let joined: Result, ()> = Err(()); + assert!(finish_uninstall_join( + &uninstalling, + &owner, + claim, + &quitting, + joined, + || recoveries.set(recoveries.get() + 1), + || panic!("a failed task must not exit"), + ) + .is_err()); + assert!(!uninstalling.load(std::sync::atomic::Ordering::SeqCst)); + assert!(quitting.load(std::sync::atomic::Ordering::SeqCst)); + assert_eq!(recoveries.get(), 1, "terminal exit must suppress recovery"); + } + + #[test] + fn uninstall_is_single_flight_and_stale_claims_cannot_clear_a_new_owner() { + let uninstalling = std::sync::atomic::AtomicBool::new(false); + let owner = std::sync::atomic::AtomicU64::new(0); + let first = claim_uninstall(&uninstalling, &owner).unwrap(); + assert_eq!( + claim_uninstall(&uninstalling, &owner).unwrap_err(), + "uninstall_in_progress" + ); + assert!(release_uninstall_claim(&uninstalling, &owner, first)); + + let second = claim_uninstall(&uninstalling, &owner).unwrap(); + assert!(!release_uninstall_claim(&uninstalling, &owner, first)); + assert!(uninstalling.load(std::sync::atomic::Ordering::SeqCst)); + assert_eq!(owner.load(std::sync::atomic::Ordering::SeqCst), second.0); + assert!(release_uninstall_claim(&uninstalling, &owner, second)); + } + + #[test] + fn worker_panic_is_caught_and_finalized_as_a_recoverable_error() { + let uninstalling = std::sync::atomic::AtomicBool::new(false); + let owner = std::sync::atomic::AtomicU64::new(0); + let quitting = std::sync::atomic::AtomicBool::new(false); + let recoveries = std::cell::Cell::new(0); + let claim = claim_uninstall(&uninstalling, &owner).unwrap(); + let result: Result<(), String> = catch_uninstall_panic(|| panic!("worker panic")); + + assert_eq!( + finish_uninstall_attempt( + &uninstalling, + &owner, + claim, + &quitting, + result, + || recoveries.set(recoveries.get() + 1), + || panic!("panic rollback must not exit"), + ), + Err("uninstall_task_panicked".to_string()) + ); + assert!(!uninstalling.load(std::sync::atomic::Ordering::SeqCst)); + assert_eq!(recoveries.get(), 1); + } + + #[test] + fn dropped_uninstall_future_does_not_cancel_retained_worker() { + let entered = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + let release = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + let finalized = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + let entered2 = entered.clone(); + let release2 = release.clone(); + let finalized2 = finalized.clone(); + + let task = tauri::async_runtime::spawn(run_retained_uninstall_task(move || { + entered2.store(true, std::sync::atomic::Ordering::SeqCst); + while !release2.load(std::sync::atomic::Ordering::SeqCst) { + std::thread::sleep(std::time::Duration::from_millis(5)); + } + finalized2.store(true, std::sync::atomic::Ordering::SeqCst); + })); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(2); + while !entered.load(std::sync::atomic::Ordering::SeqCst) + && std::time::Instant::now() < deadline + { + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert!(entered.load(std::sync::atomic::Ordering::SeqCst)); + task.abort(); + release.store(true, std::sync::atomic::Ordering::SeqCst); + + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(2); + while !finalized.load(std::sync::atomic::Ordering::SeqCst) + && std::time::Instant::now() < deadline + { + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert!(finalized.load(std::sync::atomic::Ordering::SeqCst)); + } + + #[test] + fn recursive_scan_work_runs_off_the_async_caller_thread() { + let caller = std::thread::current().id(); + let worker = tauri::async_runtime::block_on(run_scan_off_thread(|| { + std::thread::current().id() + })) + .unwrap(); + assert_ne!(worker, caller); + } + #[test] fn refuses_root_home_and_foreign_paths() { let home = PathBuf::from("/Users/someone"); diff --git a/frontend/src-tauri/tests/backend_lifecycle.rs b/frontend/src-tauri/tests/backend_lifecycle.rs index 2f56b0860..c8cf76ce3 100644 --- a/frontend/src-tauri/tests/backend_lifecycle.rs +++ b/frontend/src-tauri/tests/backend_lifecycle.rs @@ -24,13 +24,41 @@ use tauri::Listener; use tauri::Manager; use app_lib::bootstrap::{ - spawn_backend_and_wait, BootstrapStage, BootstrapState, LogPayload, - set_backend_kill_intended, + run_streaming, set_backend_kill_intended, spawn_backend_and_wait, with_backend_stopped, + BootstrapStage, BootstrapState, LogPayload, +}; +use app_lib::uninstall::{purge_uninstall_targets, UninstallTarget}; +use app_lib::{ + shutdown_backend_for_exit, AppFlags, AttachedHealthState, BackendState, + CaptureDispatchState, }; -use app_lib::{AppFlags, BackendState, CaptureDispatchState}; static HARNESS: Mutex<()> = Mutex::new(()); +#[cfg(unix)] +static SCENARIO_TERM_REQUESTED: AtomicBool = AtomicBool::new(false); + +#[cfg(unix)] +extern "C" fn scenario_term_handler(_: libc::c_int) { + SCENARIO_TERM_REQUESTED.store(true, std::sync::atomic::Ordering::SeqCst); +} + +#[cfg(unix)] +fn finish_graceful_scenario_shutdown() -> bool { + if !SCENARIO_TERM_REQUESTED.load(std::sync::atomic::Ordering::SeqCst) { + return false; + } + if let Ok(path) = std::env::var("OMNIVOICE_SCENARIO_SHUTDOWN_SENTINEL") { + let _ = std::fs::write(path, b"clean"); + } + true +} + +#[cfg(not(unix))] +fn finish_graceful_scenario_shutdown() -> bool { + false +} + // ── Scenario child ──────────────────────────────────────────────────────── /// Not a real test: when `OMNIVOICE_SCENARIO` is set, this plays the backend @@ -48,9 +76,101 @@ fn scenario_child() { Ok(_) => {} Err(_) => return, } + if std::env::var("OMNIVOICE_SCENARIO_DRAIN_WRAPPER").as_deref() == Ok("1") { + #[cfg(unix)] + unsafe { + assert!(libc::setsid() >= 0, "drain wrapper failed to escape outer group"); + } + if let Some(path) = std::env::var_os("OMNIVOICE_SCENARIO_DRAIN_WRAPPER_LOG") { + std::fs::write(path, std::process::id().to_string()) + .expect("record drain wrapper pid"); + } + #[cfg(unix)] + unsafe { + libc::raise(libc::SIGSTOP); + } + return; + } + if std::env::var("OMNIVOICE_SCENARIO_DESCENDANT").as_deref() == Ok("1") { + #[cfg(unix)] + unsafe { + if std::env::var_os("OMNIVOICE_SCENARIO_DESCENDANT_SETSID").is_some() + && std::env::var("OMNIVOICE_DESKTOP_CONTAINED").as_deref() != Ok("1") + { + assert!( + libc::setsid() >= 0, + "scenario descendant failed to escape session" + ); + } + // Exercise the bounded SIGKILL fallback: the backend parent still + // handles SIGTERM and writes its cleanup sentinel, while this + // engine-like descendant deliberately ignores the graceful phase. + libc::signal(libc::SIGTERM, libc::SIG_IGN); + } + if let Some(ready) = std::env::var_os("OMNIVOICE_SCENARIO_DESCENDANT_READY") { + std::fs::write(ready, b"ready").expect("record escaped descendant readiness"); + } + std::thread::sleep(Duration::from_secs(600)); + return; + } let get = |k: &str| std::env::var(k).unwrap_or_default(); let get_ms = |k: &str| get(k).parse::().ok(); + let spawn_log = get("OMNIVOICE_SCENARIO_SPAWN_LOG"); + if !spawn_log.is_empty() { + let mut file = std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(spawn_log) + .expect("open scenario spawn log"); + writeln!(file, "{}", std::process::id()).expect("record scenario child"); + } + + let descendant_log = get("OMNIVOICE_SCENARIO_DESCENDANT_LOG"); + if !descendant_log.is_empty() { + let spawn_descendant = move || { + let trigger = get("OMNIVOICE_SCENARIO_DESCENDANT_TRIGGER"); + while !trigger.is_empty() && !std::path::Path::new(&trigger).exists() { + std::thread::sleep(Duration::from_millis(10)); + } + let exe = std::env::current_exe().expect("current test executable"); + let child = std::process::Command::new(exe) + .args(["scenario_child", "--exact", "--nocapture"]) + .env("OMNIVOICE_SCENARIO_DESCENDANT", "1") + .spawn() + .expect("spawn scenario descendant"); + std::fs::write(descendant_log, child.id().to_string()) + .expect("record scenario descendant"); + drop(child); + }; + if get("OMNIVOICE_SCENARIO_DESCENDANT_TRIGGER").is_empty() { + spawn_descendant(); + } else { + std::thread::spawn(spawn_descendant); + } + } + + let drain_wrapper_log = get("OMNIVOICE_SCENARIO_DRAIN_WRAPPER_LOG"); + if !drain_wrapper_log.is_empty() { + let exe = std::env::current_exe().expect("current test executable"); + std::process::Command::new(exe) + .args(["scenario_child", "--exact", "--nocapture"]) + .env("OMNIVOICE_SCENARIO_DRAIN_WRAPPER", "1") + .spawn() + .expect("spawn escaped drain wrapper"); + } + + #[cfg(unix)] + if !get("OMNIVOICE_SCENARIO_SHUTDOWN_SENTINEL").is_empty() { + SCENARIO_TERM_REQUESTED.store(false, std::sync::atomic::Ordering::SeqCst); + unsafe { + libc::signal( + libc::SIGTERM, + scenario_term_handler as *const () as libc::sighandler_t, + ); + } + } + if let Some(delay) = get_ms("OMNIVOICE_SCENARIO_START_DELAY_MS") { std::thread::sleep(Duration::from_millis(delay)); } @@ -61,7 +181,14 @@ fn scenario_child() { if let Some(serve_ms) = get_ms("OMNIVOICE_SCENARIO_SERVE_MS") { let port: u16 = get("OMNIVOICE_PORT").parse().expect("OMNIVOICE_PORT"); let progress_only = get("OMNIVOICE_SCENARIO_PROGRESS_ONLY") == "1"; - let listener = std::net::TcpListener::bind(("127.0.0.1", port)).expect("bind scenario port"); + let listener = match std::net::TcpListener::bind(("127.0.0.1", port)) { + Ok(listener) => listener, + Err(error) if error.kind() == std::io::ErrorKind::AddrInUse => { + eprintln!("FATAL: scenario backend could not bind port {port}: {error}"); + std::process::exit(app_lib::backend::EXIT_PORT_IN_USE); + } + Err(error) => panic!("bind scenario port: {error}"), + }; listener.set_nonblocking(true).unwrap(); let deadline = if serve_ms == 0 { None @@ -69,6 +196,9 @@ fn scenario_child() { Some(Instant::now() + Duration::from_millis(serve_ms)) }; loop { + if finish_graceful_scenario_shutdown() { + return; + } if let Some(d) = deadline { if Instant::now() >= d { break; @@ -80,7 +210,10 @@ fn scenario_child() { let _ = stream.set_read_timeout(Some(Duration::from_millis(200))); let n = stream.read(&mut buf).unwrap_or(0); let req = String::from_utf8_lossy(&buf[..n]); - let resp = if req.starts_with("GET /startup/progress") { + let resp = if get("OMNIVOICE_SCENARIO_FOREIGN") == "1" { + "HTTP/1.1 200 OK\r\nContent-Length: 21\r\n\r\n{\"service\":\"foreign\"}" + .to_string() + } else if req.starts_with("GET /startup/progress") { let body = r#"{"status": "starting", "step": "ml_imports", "label": "Loading ML runtime (PyTorch)_"}"#; format!( "HTTP/1.1 200 OK\r\nx-omnivoice-backend: 0.0.0\r\nContent-Length: {}\r\n\r\n{}", @@ -89,8 +222,16 @@ fn scenario_child() { } else if progress_only { "HTTP/1.1 503 X\r\nContent-Length: 0\r\n\r\n".to_string() } else if req.starts_with("GET /system/info") { - let body = r#"{"data_dir": "/x", "app_version": "0.0.0"}"#; + let body = format!( + r#"{{"data_dir": "/x", "app_version": "{}"}}"#, + env!("CARGO_PKG_VERSION") + ); format!("HTTP/1.1 200 OK\r\nContent-Length: {}\r\n\r\n{}", body.len(), body) + } else if req.starts_with("GET /profiles") + && std::env::var_os("OMNIVOICE_SCENARIO_HEALTH_FAIL_FILE") + .is_some_and(|path| std::path::Path::new(&path).exists()) + { + "HTTP/1.1 503 X\r\nContent-Length: 0\r\n\r\n".to_string() } else { "HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n[]".to_string() }; @@ -117,6 +258,14 @@ fn scenario_child() { if let Some(code) = get_ms("OMNIVOICE_SCENARIO_EXIT") { std::process::exit(code as i32); } + if !get("OMNIVOICE_SCENARIO_SHUTDOWN_SENTINEL").is_empty() { + loop { + if finish_graceful_scenario_shutdown() { + return; + } + std::thread::sleep(Duration::from_millis(20)); + } + } // Scripted to serve forever / be killed externally: idle out. std::thread::sleep(Duration::from_secs(600)); } @@ -129,11 +278,19 @@ struct Scenario<'a> { signal9: bool, serve_ms: Option, progress_only: bool, + foreign: bool, } impl Default for Scenario<'_> { fn default() -> Self { - Scenario { stderr: "", exit: None, signal9: false, serve_ms: None, progress_only: false } + Scenario { + stderr: "", + exit: None, + signal9: false, + serve_ms: None, + progress_only: false, + foreign: false, + } } } @@ -144,12 +301,31 @@ const SCENARIO_ENV: &[&str] = &[ "OMNIVOICE_SCENARIO_SIGNAL", "OMNIVOICE_SCENARIO_SERVE_MS", "OMNIVOICE_SCENARIO_PROGRESS_ONLY", + "OMNIVOICE_SCENARIO_FOREIGN", "OMNIVOICE_SCENARIO_START_DELAY_MS", + "OMNIVOICE_SCENARIO_SPAWN_LOG", + "OMNIVOICE_SCENARIO_DESCENDANT", + "OMNIVOICE_SCENARIO_DESCENDANT_LOG", + "OMNIVOICE_SCENARIO_DESCENDANT_SETSID", + "OMNIVOICE_SCENARIO_DESCENDANT_READY", + "OMNIVOICE_SCENARIO_DESCENDANT_TRIGGER", + "OMNIVOICE_SCENARIO_SHUTDOWN_SENTINEL", + "OMNIVOICE_SCENARIO_DRAIN_WRAPPER", + "OMNIVOICE_SCENARIO_DRAIN_WRAPPER_LOG", + "OMNIVOICE_SCENARIO_HEALTH_FAIL_FILE", + "OMNIVOICE_TEST_BEFORE_TRACK_ENTERED", + "OMNIVOICE_TEST_BEFORE_TRACK_RELEASE", + "OMNIVOICE_TEST_AFTER_TRACK_ENTERED", + "OMNIVOICE_TEST_AFTER_TRACK_RELEASE", "OMNIVOICE_BACKEND_CMD", "OMNIVOICE_LOG_DIR", "OMNIVOICE_PORT", "OMNIVOICE_STARTUP_BUDGET_S", "OMNIVOICE_SUPERVISOR_POLL_MS", + "OMNIVOICE_ATTACHED_FAILURE_GRACE_MS", + "OMNIVOICE_TEST_FORCE_STOP_ERROR", + "OMNIVOICE_TEST_FORCE_INCOMPLETE_STOP_ERROR", + "OMNIVOICE_TEST_NESTED_DRAIN_TIMEOUT_MS", ]; struct TestApp { @@ -179,6 +355,7 @@ impl TestApp { std::env::set_var("OMNIVOICE_PORT", port.to_string()); std::env::set_var("OMNIVOICE_STARTUP_BUDGET_S", "6"); std::env::set_var("OMNIVOICE_SUPERVISOR_POLL_MS", "100"); + std::env::set_var("OMNIVOICE_ATTACHED_FAILURE_GRACE_MS", "300"); let exe = std::env::current_exe().expect("current_exe"); std::env::set_var( @@ -209,13 +386,25 @@ impl TestApp { if scenario.progress_only { std::env::set_var("OMNIVOICE_SCENARIO_PROGRESS_ONLY", "1"); } + if scenario.foreign { + std::env::set_var("OMNIVOICE_SCENARIO_FOREIGN", "1"); + } let app = tauri::test::mock_builder() .build(tauri::test::mock_context(tauri::test::noop_assets())) .expect("mock app"); - app.manage(BackendState { process: Mutex::new(None), spawned_at: Mutex::new(None) }); + app.manage(BackendState { + lifecycle: Mutex::new(()), + process: Mutex::new(None), + owned_tree: Mutex::new(None), + attached: AtomicBool::new(false), + attached_health: Mutex::new(AttachedHealthState::default()), + spawned_at: Mutex::new(None), + }); app.manage(AppFlags { quitting: AtomicBool::new(false), + uninstalling: AtomicBool::new(false), + uninstall_owner: std::sync::atomic::AtomicU64::new(0), dictating: AtomicBool::new(false), capture: Mutex::new(CaptureDispatchState::default()), output: app_lib::dictation_output::DictationOutput::default(), @@ -273,6 +462,18 @@ impl TestApp { } } + /// Model a real root crash: signal through Child's stable handle but leave + /// the zombie unreaped so its process-group identity cannot be reused + /// before the supervisor drains every descendant. + fn signal_tracked_child(&self) { + let state = self.app.state::(); + if let Ok(mut guard) = state.process.lock() { + if let Some(child) = guard.as_mut() { + let _ = child.kill(); + } + }; + } + fn quit(&self) { self.app .state::() @@ -316,8 +517,808 @@ fn join_with_timeout(h: std::thread::JoinHandle<()>, timeout: Duration, what: &s let _ = h.join(); } +fn recorded_spawn_count(path: &std::path::Path) -> usize { + std::fs::read_to_string(path) + .unwrap_or_default() + .lines() + .filter(|line| !line.trim().is_empty()) + .count() +} + +fn process_is_alive(pid: u32) -> bool { + let pid = sysinfo::Pid::from_u32(pid); + let mut system = sysinfo::System::new(); + system.refresh_processes(sysinfo::ProcessesToUpdate::Some(&[pid]), false); + system.process(pid).is_some() +} + +fn force_kill_pid_for_cleanup(pid: u32) { + #[cfg(unix)] + unsafe { + libc::kill(pid as i32, libc::SIGKILL); + } + #[cfg(windows)] + { + let pid = pid.to_string(); + let _ = app_lib::tools::no_window( + std::process::Command::new("taskkill") + .args(["/PID", pid.as_str(), "/T", "/F"]), + ) + .status(); + } +} + // ── Scenarios ───────────────────────────────────────────────────────────── +/// #1635 — launch bootstrap and Retry used to probe an empty port together, +/// spawn independently, and overwrite the one tracked child. The untracked +/// winner stayed healthy while the loser wrote a false port-conflict crash. +#[test] +fn concurrent_bootstrap_and_retry_share_one_backend_child() { + let t = TestApp::new(&Scenario { + serve_ms: Some(0), // serve until the harness stops the child + ..Default::default() + }); + let spawn_log = t._logdir.path().join("scenario-spawns.log"); + std::env::set_var("OMNIVOICE_SCENARIO_SPAWN_LOG", &spawn_log); + // Hold both real child processes before bind so both launch owners have + // time to reach spawn on the broken implementation. + std::env::set_var("OMNIVOICE_SCENARIO_START_DELAY_MS", "300"); + + let gate = Arc::new(std::sync::Barrier::new(3)); + let launch = |gate: Arc, handle, stage| { + std::thread::spawn(move || { + gate.wait(); + spawn_backend_and_wait(&handle, &stage); + }) + }; + let bootstrap = launch(gate.clone(), t.handle(), t.stage.clone()); + let retry = launch(gate.clone(), t.handle(), t.stage.clone()); + gate.wait(); + + assert!( + wait_until(Duration::from_secs(20), || matches!( + t.stage_snapshot(), + BootstrapStage::Ready + )), + "concurrent launch never produced a healthy backend" + ); + assert!( + wait_until(Duration::from_secs(10), || bootstrap.is_finished() || retry.is_finished()), + "the losing launch owner did not attach to the healthy child" + ); + assert_eq!( + recorded_spawn_count(&spawn_log), + 1, + "bootstrap + Retry must create exactly one OS child" + ); + assert!( + app_lib::backend::backend_ready( + std::env::var("OMNIVOICE_PORT").unwrap().parse().unwrap() + ), + "the shared child must pass both readiness probes" + ); + assert!( + t.markers().markers.is_empty(), + "a serialized launch must not manufacture a bind-crash marker" + ); + + t.quit(); + t.kill_tracked_child(); + join_with_timeout(bootstrap, Duration::from_secs(10), "concurrent bootstrap shutdown"); + join_with_timeout(retry, Duration::from_secs(10), "concurrent retry shutdown"); +} + +/// A healthy same-version listener may predate the current launch (for +/// example after the webview shell crashed). Attaching without owning its PID +/// used to return Ready with no supervisor, so its next death was permanent. +#[test] +fn healthy_external_backend_is_attached_with_supervision_and_replaced_after_death() { + let t = TestApp::new(&Scenario { + serve_ms: Some(0), + ..Default::default() + }); + let spawn_log = t._logdir.path().join("scenario-attached-spawns.log"); + std::env::set_var("OMNIVOICE_SCENARIO_SPAWN_LOG", &spawn_log); + let exe = std::env::current_exe().expect("current test executable"); + let mut external = std::process::Command::new(exe) + .args(["scenario_child", "--exact", "--nocapture"]) + .spawn() + .expect("spawn external healthy backend"); + let port = std::env::var("OMNIVOICE_PORT").unwrap().parse().unwrap(); + assert!( + wait_until(Duration::from_secs(10), || { + app_lib::backend::backend_ready(port) && recorded_spawn_count(&spawn_log) == 1 + }), + "external backend never became healthy" + ); + + let bootstrap = t.run_bootstrap(); + assert!( + wait_until(Duration::from_secs(10), || { + let state = t.app.state::(); + matches!(t.stage_snapshot(), BootstrapStage::Ready) + && state.attached.load(std::sync::atomic::Ordering::SeqCst) + && state.process.lock().unwrap().is_none() + && state.owned_tree.lock().unwrap().is_none() + }), + "launch reported Ready without health-supervising the external backend" + ); + + external.kill().expect("kill attached backend"); + let _ = external.wait(); + assert!( + wait_until(Duration::from_secs(20), || { + recorded_spawn_count(&spawn_log) == 2 + && app_lib::backend::backend_ready(port) + && matches!(t.stage_snapshot(), BootstrapStage::Ready) + }), + "attached backend death was not replaced by a healthy supervised child" + ); + + shutdown_backend_for_exit(&t.handle()); + join_with_timeout(bootstrap, Duration::from_secs(10), "attached-backend shutdown"); + assert!(!app_lib::backend::port_in_use(port)); +} + +#[test] +fn attached_backend_health_grace_recovers_without_false_crash_or_port_failure() { + let t = TestApp::new(&Scenario { + serve_ms: Some(0), + ..Default::default() + }); + let spawn_log = t._logdir.path().join("scenario-attached-health-spawns.log"); + let fail_file = t._logdir.path().join("scenario-attached-health-fail"); + std::env::set_var("OMNIVOICE_SCENARIO_SPAWN_LOG", &spawn_log); + std::env::set_var("OMNIVOICE_SCENARIO_HEALTH_FAIL_FILE", &fail_file); + let exe = std::env::current_exe().expect("current test executable"); + let mut external = std::process::Command::new(exe) + .args(["scenario_child", "--exact", "--nocapture"]) + .spawn() + .expect("spawn external healthy backend"); + let external_pid = external.id(); + let port = std::env::var("OMNIVOICE_PORT").unwrap().parse().unwrap(); + assert!(wait_until(Duration::from_secs(10), || { + app_lib::backend::backend_ready(port) + })); + + let restarts = t.record_events("backend-restarting"); + let bootstrap = t.run_bootstrap(); + assert!(wait_until(Duration::from_secs(10), || { + t.app + .state::() + .attached + .load(std::sync::atomic::Ordering::SeqCst) + && matches!(t.stage_snapshot(), BootstrapStage::Ready) + })); + let markers_before = t.markers().markers.len(); + + std::fs::write(&fail_file, b"fail deep health").unwrap(); + std::thread::sleep(Duration::from_millis(1200)); + std::fs::remove_file(&fail_file).unwrap(); + assert!(wait_until(Duration::from_secs(5), || { + app_lib::backend::backend_ready(port) + && t + .app + .state::() + .attached + .load(std::sync::atomic::Ordering::SeqCst) + && t + .app + .state::() + .attached_health + .lock() + .unwrap() + .failures + == 0 + })); + + assert!(process_is_alive(external_pid)); + assert_eq!(recorded_spawn_count(&spawn_log), 1); + assert_eq!(t.markers().markers.len(), markers_before); + assert!(restarts.lock().unwrap().is_empty()); + assert!(matches!(t.stage_snapshot(), BootstrapStage::Ready)); + assert_eq!( + t.app + .state::() + .attached_health + .lock() + .unwrap() + .failures, + 0, + "a healthy sample must reset the consecutive-failure policy" + ); + + shutdown_backend_for_exit(&t.handle()); + join_with_timeout(bootstrap, Duration::from_secs(10), "attached health recovery"); + assert!(process_is_alive(external_pid)); + external.kill().unwrap(); + let _ = external.wait(); +} + +/// A matching API is sufficient for a safe attachment, never for ownership. +/// Terminal desktop teardown must leave the external process untouched. +#[test] +fn healthy_external_attachment_is_not_killed_on_desktop_exit() { + let t = TestApp::new(&Scenario { + serve_ms: Some(0), + ..Default::default() + }); + let spawn_log = t._logdir.path().join("scenario-safe-attachment.log"); + std::env::set_var("OMNIVOICE_SCENARIO_SPAWN_LOG", &spawn_log); + let exe = std::env::current_exe().expect("current test executable"); + let mut external = std::process::Command::new(exe) + .args(["scenario_child", "--exact", "--nocapture"]) + .spawn() + .expect("spawn external healthy backend"); + let external_pid = external.id(); + let port = std::env::var("OMNIVOICE_PORT").unwrap().parse().unwrap(); + assert!(wait_until(Duration::from_secs(10), || { + app_lib::backend::backend_ready(port) + })); + + let bootstrap = t.run_bootstrap(); + assert!(wait_until(Duration::from_secs(10), || { + t.app + .state::() + .attached + .load(std::sync::atomic::Ordering::SeqCst) + })); + shutdown_backend_for_exit(&t.handle()); + join_with_timeout(bootstrap, Duration::from_secs(10), "safe external detach"); + + assert!(process_is_alive(external_pid)); + assert!(app_lib::backend::port_in_use(port)); + external.kill().unwrap(); + let _ = external.wait(); +} + +/// A backend may launch an engine after Ready and crash immediately afterwards +/// (before any supervisor poll). Launch-time containment, rather than sampled +/// ancestry, must drain that late child before a replacement starts. +#[test] +fn crash_replacement_and_exit_terminate_descendants_from_both_generations() { + let t = TestApp::new(&Scenario { + serve_ms: Some(0), + ..Default::default() + }); + let spawn_log = t._logdir.path().join("scenario-crash-tree-spawns.log"); + let descendant_log = t._logdir.path().join("scenario-crash-tree-descendant.log"); + let descendant_ready = t._logdir.path().join("scenario-crash-tree-descendant-ready"); + let descendant_trigger = t._logdir.path().join("scenario-crash-tree-trigger"); + std::env::set_var("OMNIVOICE_SCENARIO_SPAWN_LOG", &spawn_log); + std::env::set_var("OMNIVOICE_SCENARIO_DESCENDANT_LOG", &descendant_log); + std::env::set_var("OMNIVOICE_SCENARIO_DESCENDANT_SETSID", "1"); + std::env::set_var("OMNIVOICE_SCENARIO_DESCENDANT_READY", &descendant_ready); + std::env::set_var("OMNIVOICE_SCENARIO_DESCENDANT_TRIGGER", &descendant_trigger); + + let bootstrap = t.run_bootstrap(); + assert!( + wait_until(Duration::from_secs(20), || { + matches!(t.stage_snapshot(), BootstrapStage::Ready) + }), + "first backend generation never became ready" + ); + std::fs::write(&descendant_trigger, b"spawn now").unwrap(); + assert!( + wait_until(Duration::from_secs(10), || descendant_ready.exists()), + "late descendant did not start after Ready" + ); + let original_descendant: u32 = std::fs::read_to_string(&descendant_log) + .unwrap() + .trim() + .parse() + .unwrap(); + assert!(process_is_alive(original_descendant)); + + t.signal_tracked_child(); + assert!( + wait_until(Duration::from_secs(20), || { + recorded_spawn_count(&spawn_log) == 2 + && matches!(t.stage_snapshot(), BootstrapStage::Ready) + && std::fs::read_to_string(&descendant_log) + .ok() + .and_then(|pid| pid.trim().parse::().ok()) + .is_some_and(|pid| pid != original_descendant) + }), + "crashed backend was not replaced with a ready generation" + ); + assert!( + wait_until(Duration::from_secs(3), || !process_is_alive(original_descendant)), + "late descendant from crashed root survived into the replacement generation" + ); + let replacement_descendant: u32 = std::fs::read_to_string(&descendant_log) + .unwrap() + .trim() + .parse() + .unwrap(); + + shutdown_backend_for_exit(&t.handle()); + join_with_timeout(bootstrap, Duration::from_secs(10), "crash-tree shutdown"); + let original_stopped = wait_until(Duration::from_secs(3), || { + !process_is_alive(original_descendant) + }); + let replacement_stopped = wait_until(Duration::from_secs(3), || { + !process_is_alive(replacement_descendant) + }); + if !original_stopped { + force_kill_pid_for_cleanup(original_descendant); + } + if !replacement_stopped { + force_kill_pid_for_cleanup(replacement_descendant); + } + assert!( + original_stopped, + "escaped descendant from crashed root {original_descendant} survived replacement and exit" + ); + assert!( + replacement_stopped, + "escaped descendant from replacement {replacement_descendant} survived exit" + ); +} + +/// A nested supervisor is intentionally outside the backend's Rust process +/// group. Teardown may complete only after its inherited drain writer closes; +/// a stopped wrapper must therefore preserve retryable handles and block the +/// caller's destructive action/replacement rather than relying on a sleep. +#[cfg(unix)] +#[test] +fn stopped_nested_wrapper_blocks_mutation_until_drain_retry() { + let t = TestApp::new(&Scenario { + serve_ms: Some(0), + ..Default::default() + }); + let wrapper_log = t._logdir.path().join("scenario-stopped-drain-wrapper.log"); + std::env::set_var("OMNIVOICE_SCENARIO_DRAIN_WRAPPER_LOG", &wrapper_log); + std::env::set_var("OMNIVOICE_TEST_NESTED_DRAIN_TIMEOUT_MS", "300"); + let bootstrap = t.run_bootstrap(); + assert!(wait_until(Duration::from_secs(20), || { + matches!(t.stage_snapshot(), BootstrapStage::Ready) && wrapper_log.exists() + })); + let wrapper_pid: u32 = std::fs::read_to_string(&wrapper_log) + .unwrap() + .trim() + .parse() + .unwrap(); + assert!(process_is_alive(wrapper_pid)); + + let mutated = Arc::new(AtomicBool::new(false)); + let mutated_first = mutated.clone(); + let first = with_backend_stopped(&t.handle(), move || { + mutated_first.store(true, std::sync::atomic::Ordering::SeqCst); + }); + let mutated_before_retry = mutated.load(std::sync::atomic::Ordering::SeqCst); + let refused = first + .as_ref() + .is_err_and(|message| message.contains("nested backend operations did not drain")); + let state = t.app.state::(); + let retryable = state.process.lock().unwrap().is_some() + && state.owned_tree.lock().unwrap().is_some(); + let wrapper_survived_timeout = process_is_alive(wrapper_pid); + + unsafe { + libc::kill(wrapper_pid as libc::pid_t, libc::SIGCONT); + } + assert!(wait_until(Duration::from_secs(5), || !process_is_alive(wrapper_pid))); + let mutated_retry = mutated.clone(); + let retry = with_backend_stopped(&t.handle(), move || { + mutated_retry.store(true, std::sync::atomic::Ordering::SeqCst); + }); + + assert!(refused, "first teardown unexpectedly completed: {first:?}"); + assert!(!mutated_before_retry, "mutation ran before nested drain EOF"); + assert!(retryable, "drain timeout discarded the stable retry handles"); + assert!(wrapper_survived_timeout, "timeout test wrapper was not still stopped"); + assert!(retry.is_ok(), "drain retry failed after wrapper exit: {retry:?}"); + assert!(mutated.load(std::sync::atomic::Ordering::SeqCst)); + assert!(state.process.lock().unwrap().is_none()); + assert!(state.owned_tree.lock().unwrap().is_none()); + + t.quit(); + join_with_timeout(bootstrap, Duration::from_secs(10), "nested drain retry"); +} + +/// A configured-port conflict is user input, not authority to terminate an +/// arbitrary application. The orphan cleanup path must leave a stable foreign +/// LISTEN owner alive even when it is the exact PID returned by lsof/netstat. +#[test] +fn orphan_cleanup_refuses_a_foreign_listener() { + let t = TestApp::new(&Scenario { + serve_ms: Some(0), + foreign: true, + ..Default::default() + }); + let spawn_log = t._logdir.path().join("scenario-foreign-listener.log"); + std::env::set_var("OMNIVOICE_SCENARIO_SPAWN_LOG", &spawn_log); + let bootstrap = t.run_bootstrap(); + let port = std::env::var("OMNIVOICE_PORT").unwrap().parse().unwrap(); + assert!( + wait_until(Duration::from_secs(10), || { + app_lib::backend::port_in_use(port) && recorded_spawn_count(&spawn_log) == 1 + }), + "foreign scenario never bound the configured port" + ); + let pid: u32 = std::fs::read_to_string(&spawn_log) + .unwrap() + .trim() + .parse() + .unwrap(); + + app_lib::backend::kill_orphan_on_port(port); + std::thread::sleep(Duration::from_millis(300)); + assert!( + process_is_alive(pid), + "orphan cleanup killed a foreign process" + ); + assert!(app_lib::backend::port_in_use(port)); + + t.quit(); + t.kill_tracked_child(); + join_with_timeout( + bootstrap, + Duration::from_secs(10), + "foreign-listener cleanup", + ); +} + +/// A stop failure happens after the tracked slot has been taken. Every +/// non-terminal caller of the lifecycle guard must therefore schedule a fresh +/// serialized launch instead of trusting the old supervisor to survive. +#[test] +fn failed_backend_stop_rearms_service_for_all_guard_callers() { + let t = TestApp::new(&Scenario { + serve_ms: Some(0), + ..Default::default() + }); + let spawn_log = t._logdir.path().join("scenario-stop-recovery.log"); + std::env::set_var("OMNIVOICE_SCENARIO_SPAWN_LOG", &spawn_log); + let bootstrap = t.run_bootstrap(); + assert!( + wait_until(Duration::from_secs(20), || matches!( + t.stage_snapshot(), + BootstrapStage::Ready + )), + "initial backend never became ready" + ); + + std::env::set_var("OMNIVOICE_TEST_FORCE_STOP_ERROR", "1"); + let action_ran = std::cell::Cell::new(false); + let result = with_backend_stopped(&t.handle(), || action_ran.set(true)); + std::env::remove_var("OMNIVOICE_TEST_FORCE_STOP_ERROR"); + assert!(result.is_err(), "fault seam did not fail the stop"); + assert!(!action_ran.get(), "caller mutation ran after a failed stop"); + assert!( + wait_until(Duration::from_secs(20), || { + recorded_spawn_count(&spawn_log) == 2 + && matches!(t.stage_snapshot(), BootstrapStage::Ready) + }), + "failed stop did not restore a supervised backend" + ); + + t.quit(); + t.kill_tracked_child(); + join_with_timeout(bootstrap, Duration::from_secs(10), "failed-stop recovery"); +} + +/// A surviving backend tree is categorically different from a recoverable +/// post-stop error: spawning a replacement could duplicate engine workers or +/// race files still held by the survivor. Keep the deliberate-kill fence up +/// and require a later explicit retry after the teardown problem is resolved. +#[test] +fn incomplete_backend_stop_does_not_spawn_a_replacement() { + let t = TestApp::new(&Scenario { + serve_ms: Some(0), + ..Default::default() + }); + let spawn_log = t._logdir.path().join("scenario-incomplete-stop.log"); + std::env::set_var("OMNIVOICE_SCENARIO_SPAWN_LOG", &spawn_log); + let bootstrap = t.run_bootstrap(); + assert!( + wait_until(Duration::from_secs(20), || matches!( + t.stage_snapshot(), + BootstrapStage::Ready + )), + "initial backend never became ready" + ); + + std::env::set_var("OMNIVOICE_TEST_FORCE_INCOMPLETE_STOP_ERROR", "1"); + let result = with_backend_stopped(&t.handle(), || ()); + std::env::remove_var("OMNIVOICE_TEST_FORCE_INCOMPLETE_STOP_ERROR"); + assert!(result.is_err(), "fault seam did not fail the stop"); + std::thread::sleep(Duration::from_millis(500)); + assert_eq!( + recorded_spawn_count(&spawn_log), + 1, + "incomplete teardown must not launch a replacement" + ); + assert!(t.app.state::().process.lock().unwrap().is_none()); + + t.quit(); + join_with_timeout(bootstrap, Duration::from_secs(10), "incomplete-stop shutdown"); +} + +/// #1635 follow-up — uninstall used to kill by port only, so a tracked child +/// still inside its pre-bind delay survived while its live environment was +/// deleted. Uninstall must wait for the launch owner, stop that exact child, +/// and keep lifecycle ownership through deletion. +#[test] +fn uninstall_waits_for_an_unbound_tracked_child_before_deleting() { + let t = TestApp::new(&Scenario { + serve_ms: Some(0), + ..Default::default() + }); + let spawn_log = t._logdir.path().join("scenario-uninstall-spawns.log"); + let descendant_log = t._logdir.path().join("scenario-uninstall-descendant.log"); + let descendant_ready = t._logdir.path().join("scenario-uninstall-descendant-ready"); + let after_track = t._logdir.path().join("uninstall-after-track-entered"); + let release_track = t._logdir.path().join("uninstall-release-after-track"); + let target_path = t._logdir.path().join("OmniVoice"); + std::fs::create_dir_all(&target_path).expect("create synthetic uninstall target"); + std::fs::write(target_path.join("live-env.txt"), b"live").expect("seed uninstall target"); + std::env::set_var("OMNIVOICE_SCENARIO_SPAWN_LOG", &spawn_log); + std::env::set_var("OMNIVOICE_SCENARIO_DESCENDANT_LOG", &descendant_log); + std::env::set_var("OMNIVOICE_SCENARIO_DESCENDANT_SETSID", "1"); + std::env::set_var("OMNIVOICE_SCENARIO_DESCENDANT_READY", &descendant_ready); + std::env::set_var("OMNIVOICE_SCENARIO_START_DELAY_MS", "5000"); + std::env::set_var("OMNIVOICE_TEST_AFTER_TRACK_ENTERED", &after_track); + std::env::set_var("OMNIVOICE_TEST_AFTER_TRACK_RELEASE", &release_track); + + let bootstrap = t.run_bootstrap(); + let reached_gate = wait_until(Duration::from_secs(10), || { + after_track.exists() && descendant_ready.exists() && recorded_spawn_count(&spawn_log) == 1 + }); + if !reached_gate { + let _ = std::fs::write(&release_track, b"release"); + } + assert!(reached_gate, "scenario child never reached the tracked pre-bind gate"); + let pid: u32 = std::fs::read_to_string(&spawn_log) + .unwrap() + .lines() + .next() + .unwrap() + .parse() + .unwrap(); + let descendant_pid: u32 = std::fs::read_to_string(&descendant_log) + .unwrap() + .trim() + .parse() + .unwrap(); + let port = std::env::var("OMNIVOICE_PORT").unwrap().parse().unwrap(); + assert!( + !app_lib::backend::port_in_use(port), + "precondition: the tracked child must still be unbound" + ); + + // Production `uninstall_purge` suppresses launches without claiming a + // terminal exit before entering this shared deletion core. + t.app + .state::() + .uninstalling + .store(true, std::sync::atomic::Ordering::SeqCst); + let app = t.handle(); + let target_string = target_path.to_string_lossy().into_owned(); + let target = UninstallTarget { + key: "env".to_string(), + path: target_string.clone(), + size_bytes: 4, + exists: true, + shared: false, + }; + let purge = std::thread::spawn(move || purge_uninstall_targets(&app, vec![target], false)); + + let purged_while_prebind = wait_until(Duration::from_secs(2), || purge.is_finished()); + let target_survived_until_launch_released = target_path.exists(); + std::fs::write(&release_track, b"release").unwrap(); + join_with_timeout(bootstrap, Duration::from_secs(10), "uninstall overlap shutdown"); + let report = purge.join().expect("purge thread panicked").expect("purge failed"); + assert!( + !purged_while_prebind && target_survived_until_launch_released, + "uninstall deleted the live environment before joining the tracked pre-bind child" + ); + assert_eq!(report.removed, vec![target_string]); + assert!(!target_path.exists(), "target must be removed after the child stops"); + assert!( + t.app.state::().process.lock().unwrap().is_none(), + "uninstall must not leave the tracked child orphaned" + ); + assert!(!process_is_alive(pid), "uninstall left backend pid {pid} orphaned"); + assert!( + !process_is_alive(descendant_pid), + "uninstall left escaped descendant pid {descendant_pid} orphaned" + ); + assert!( + wait_until(Duration::from_secs(5), || !app_lib::backend::port_in_use(port)), + "the stopped child must release its backend port" + ); + assert_eq!(recorded_spawn_count(&spawn_log), 1, "uninstall must not spawn a replacement"); + assert!(t.markers().markers.is_empty(), "an intentional uninstall is not a crash"); +} + +/// #1635 production-exit follow-up — ExitRequested used to inspect only the +/// process slot, outside lifecycle ownership. If exit landed after OS spawn +/// but before tracking, bootstrap installed the child afterwards and left it +/// alive. The production exit path must join launch, then stop that child. +#[test] +fn production_exit_joins_a_prebind_spawn_and_leaves_no_orphan() { + let t = TestApp::new(&Scenario { + serve_ms: Some(0), + ..Default::default() + }); + let spawn_log = t._logdir.path().join("scenario-exit-spawns.log"); + let before_track = t._logdir.path().join("before-track-entered"); + let release_track = t._logdir.path().join("release-before-track"); + std::env::set_var("OMNIVOICE_SCENARIO_SPAWN_LOG", &spawn_log); + std::env::set_var("OMNIVOICE_SCENARIO_START_DELAY_MS", "1500"); + std::env::set_var("OMNIVOICE_TEST_BEFORE_TRACK_ENTERED", &before_track); + std::env::set_var("OMNIVOICE_TEST_BEFORE_TRACK_RELEASE", &release_track); + + let bootstrap = t.run_bootstrap(); + let reached_gate = wait_until(Duration::from_secs(10), || { + before_track.exists() && recorded_spawn_count(&spawn_log) == 1 + }); + if !reached_gate { + let _ = std::fs::write(&release_track, b"release"); + } + assert!(reached_gate, "backend never reached the spawned-but-untracked test gate"); + let pid: u32 = std::fs::read_to_string(&spawn_log) + .unwrap() + .lines() + .next() + .unwrap() + .parse() + .unwrap(); + let port = std::env::var("OMNIVOICE_PORT").unwrap().parse().unwrap(); + let child_is_prebind = !app_lib::backend::port_in_use(port); + if !child_is_prebind { + let _ = std::fs::write(&release_track, b"release"); + } + assert!(child_is_prebind, "precondition: child is pre-bind"); + + let app = t.handle(); + let shutdown = std::thread::spawn(move || shutdown_backend_for_exit(&app)); + let quitting = wait_until(Duration::from_secs(5), || { + t.app + .state::() + .quitting + .load(std::sync::atomic::Ordering::SeqCst) + }); + if quitting { + assert!(!shutdown.is_finished(), "exit must join the active lifecycle owner"); + } + std::fs::write(&release_track, b"release").unwrap(); + assert!(quitting, "production exit did not raise quitting before teardown"); + + join_with_timeout(bootstrap, Duration::from_secs(10), "production exit bootstrap"); + join_with_timeout(shutdown, Duration::from_secs(10), "production exit teardown"); + assert!( + t.app.state::().process.lock().unwrap().is_none(), + "production exit must clear the tracked child" + ); + assert!( + wait_until(Duration::from_secs(5), || !process_is_alive(pid)), + "production exit left backend pid {pid} orphaned" + ); + assert!(!app_lib::backend::port_in_use(port), "orphan must not claim the port later"); + assert_eq!(recorded_spawn_count(&spawn_log), 1, "shutdown must not respawn"); + assert!(t.markers().markers.is_empty(), "intentional exit is not a crash"); +} + +/// Graceful-first shutdown must let the backend's lifespan cleanup run, then +/// prove the whole backend process tree is gone — killing only the tracked +/// parent leaves subprocess-isolated engines alive and keeps Windows files +/// locked. +#[cfg(unix)] +#[test] +fn production_exit_runs_cleanup_and_stops_backend_descendants() { + let t = TestApp::new(&Scenario { + serve_ms: Some(0), + ..Default::default() + }); + let spawn_log = t._logdir.path().join("scenario-graceful-parent.log"); + let descendant_log = t._logdir.path().join("scenario-graceful-descendant.log"); + let descendant_ready = t._logdir.path().join("scenario-graceful-descendant-ready"); + let sentinel = t._logdir.path().join("scenario-clean-shutdown"); + std::env::set_var("OMNIVOICE_SCENARIO_SPAWN_LOG", &spawn_log); + std::env::set_var("OMNIVOICE_SCENARIO_DESCENDANT_LOG", &descendant_log); + std::env::set_var("OMNIVOICE_SCENARIO_DESCENDANT_SETSID", "1"); + std::env::set_var("OMNIVOICE_SCENARIO_DESCENDANT_READY", &descendant_ready); + std::env::set_var("OMNIVOICE_SCENARIO_SHUTDOWN_SENTINEL", &sentinel); + + let bootstrap = t.run_bootstrap(); + assert!( + wait_until(Duration::from_secs(20), || matches!( + t.stage_snapshot(), + BootstrapStage::Ready + ) && descendant_ready.exists()), + "backend tree never became ready" + ); + let parent_pid: u32 = std::fs::read_to_string(&spawn_log).unwrap().trim().parse().unwrap(); + let descendant_pid: u32 = std::fs::read_to_string(&descendant_log) + .unwrap() + .trim() + .parse() + .unwrap(); + let port = std::env::var("OMNIVOICE_PORT").unwrap().parse().unwrap(); + + shutdown_backend_for_exit(&t.handle()); + join_with_timeout(bootstrap, Duration::from_secs(10), "graceful process-tree shutdown"); + let cleaned = sentinel.exists(); + let descendant_stopped = wait_until(Duration::from_secs(3), || !process_is_alive(descendant_pid)); + if !descendant_stopped { + force_kill_pid_for_cleanup(descendant_pid); + } + + assert!(cleaned, "SIGTERM must run the backend lifespan cleanup sentinel"); + assert!(!process_is_alive(parent_pid), "tracked backend parent survived shutdown"); + assert!(descendant_stopped, "backend descendant pid {descendant_pid} survived shutdown"); + assert!(!app_lib::backend::port_in_use(port), "backend port survived tree shutdown"); + assert_eq!(recorded_spawn_count(&spawn_log), 1, "shutdown must not respawn"); + assert!(t.markers().markers.is_empty(), "graceful exit is not a crash"); +} + +/// First-run `uv venv` / `uv sync` runs while launch owns lifecycle. Its wait +/// must notice quitting, terminate and reap the whole subprocess tree, and +/// release lifecycle so production ExitRequested cannot hang indefinitely. +#[test] +fn production_exit_interrupts_a_running_bootstrap_install_tree() { + let t = TestApp::new(&Scenario::default()); + let spawn_log = t._logdir.path().join("scenario-install-parent.log"); + let descendant_log = t._logdir.path().join("scenario-install-descendant.log"); + let descendant_ready = t._logdir.path().join("scenario-install-descendant-ready"); + std::env::set_var("OMNIVOICE_SCENARIO_SPAWN_LOG", &spawn_log); + std::env::set_var("OMNIVOICE_SCENARIO_DESCENDANT_LOG", &descendant_log); + std::env::set_var("OMNIVOICE_SCENARIO_DESCENDANT_SETSID", "1"); + std::env::set_var("OMNIVOICE_SCENARIO_DESCENDANT_READY", &descendant_ready); + + let outcome = Arc::new(Mutex::new(None)); + let outcome2 = outcome.clone(); + let app = t.handle(); + let installer = std::thread::spawn(move || { + let state = app.state::(); + let _lifecycle = state.lifecycle.lock().unwrap_or_else(|e| e.into_inner()); + let exe = std::env::current_exe().expect("current test executable"); + let mut cmd = std::process::Command::new(exe); + cmd.args(["scenario_child", "--exact", "--nocapture"]); + let result = run_streaming(&app, "installing_deps", &mut cmd); + *outcome2.lock().unwrap() = Some(match result { + Ok(status) => Ok(status.success()), + Err(error) => Err(error.kind()), + }); + }); + assert!( + wait_until(Duration::from_secs(10), || { + spawn_log.exists() && descendant_ready.exists() + }), + "bootstrap install tree never started" + ); + let parent_pid: u32 = std::fs::read_to_string(&spawn_log).unwrap().trim().parse().unwrap(); + let descendant_pid: u32 = std::fs::read_to_string(&descendant_log) + .unwrap() + .trim() + .parse() + .unwrap(); + + let app = t.handle(); + let shutdown = std::thread::spawn(move || shutdown_backend_for_exit(&app)); + let exit_completed = wait_until(Duration::from_secs(3), || shutdown.is_finished()); + if !exit_completed { + force_kill_pid_for_cleanup(descendant_pid); + force_kill_pid_for_cleanup(parent_pid); + } + join_with_timeout(installer, Duration::from_secs(10), "cancelled bootstrap install"); + join_with_timeout(shutdown, Duration::from_secs(10), "exit during bootstrap install"); + + assert!(exit_completed, "ExitRequested blocked behind a bootstrap subprocess wait"); + assert_eq!( + *outcome.lock().unwrap(), + Some(Err(std::io::ErrorKind::Interrupted)), + "quitting must interrupt the bootstrap subprocess" + ); + assert!(!process_is_alive(parent_pid), "bootstrap subprocess parent survived exit"); + assert!(!process_is_alive(descendant_pid), "bootstrap subprocess descendant survived exit"); + assert!(t.markers().markers.is_empty(), "cancelled install is not a backend crash"); +} + /// S1 — the backend exits EXIT_PORT_IN_USE: the user must read a port /// conflict (in the exact phrasing BootstrapSplash.detectHints localizes), /// not a traceback whose one meaningful line is an OS-translated errno. @@ -503,7 +1504,7 @@ fn deliberate_kill_yields_without_a_crash_marker() { ); let before = t.markers().markers.len(); set_backend_kill_intended(true); - t.kill_tracked_child(); + t.signal_tracked_child(); join_with_timeout(h, Duration::from_secs(30), "deliberate kill"); assert_eq!(t.markers().markers.len(), before, "no marker for an intentional kill"); diff --git a/frontend/src/components/FirstRunSetup.jsx b/frontend/src/components/FirstRunSetup.jsx index 8fbbc2058..1052cfd48 100644 --- a/frontend/src/components/FirstRunSetup.jsx +++ b/frontend/src/components/FirstRunSetup.jsx @@ -413,11 +413,18 @@ export default function FirstRunSetup() { // normal bootstrap progress UI takes over. Nothing to do here. } catch (e) { if (mounted.current) { - setServerError(String(e)); + const message = e instanceof Error ? e.message : String(e); + setServerError( + message === 'backend_stop_failed' + ? t('firstrun.backend_stop_failed') + : message === 'setup_task_failed' + ? t('bootstrap.unknown_error') + : message, + ); setSubmitting(false); } } - }, [plan, submitting, locale]); + }, [plan, submitting, locale, t]); if (!setup || !plan) { return ( diff --git a/frontend/src/components/settings/UninstallPanel.jsx b/frontend/src/components/settings/UninstallPanel.jsx index e0d407dba..f6f7e2c19 100644 --- a/frontend/src/components/settings/UninstallPanel.jsx +++ b/frontend/src/components/settings/UninstallPanel.jsx @@ -102,10 +102,11 @@ export default function UninstallPanel() { await invoke('quit_app').catch(() => {}); } catch (e) { setBusy(false); + const message = e instanceof Error ? e.message : String(e); toast.error( t('settings.uninstall_failed', { defaultValue: 'Could not remove the data: {{message}}', - message: e?.message || String(e), + message: message === 'uninstall_task_failed' ? t('bootstrap.unknown_error') : message, }), ); } diff --git a/frontend/src/i18n/locales/ar.json b/frontend/src/i18n/locales/ar.json index 105b17a30..32f9bee6e 100644 --- a/frontend/src/i18n/locales/ar.json +++ b/frontend/src/i18n/locales/ar.json @@ -2198,6 +2198,7 @@ "retry_load": "أعد المحاولة" }, "firstrun": { + "backend_stop_failed": "تعذّر على VoiceStudio إيقاف الواجهة الخلفية السابقة بأمان. أغلق أي نافذة أو عملية أخرى لـ VoiceStudio، ثم أعد المحاولة.", "loading": "جارٍ تجهيز الإعداد…", "title": "إعداد VoiceStudio", "subtitle": "لم يتم تثبيت أي شيء بعد — راجع أماكن حفظ كل شيء ثم ابدأ. يمكنك تغييرها لاحقًا من الإعدادات.", diff --git a/frontend/src/i18n/locales/de.json b/frontend/src/i18n/locales/de.json index 111790fba..138e070d4 100644 --- a/frontend/src/i18n/locales/de.json +++ b/frontend/src/i18n/locales/de.json @@ -2198,6 +2198,7 @@ "retry_load": "Versuchen Sie es noch einmal" }, "firstrun": { + "backend_stop_failed": "VoiceStudio konnte das vorherige Backend nicht sicher beenden. Schließe alle anderen VoiceStudio-Fenster oder -Prozesse und versuche es erneut.", "loading": "Einrichtung wird vorbereitet…", "title": "VoiceStudio einrichten", "subtitle": "Noch ist nichts installiert – prüfe, wo alles gespeichert wird, und starte dann. Später in den Einstellungen änderbar.", diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 2c76028e7..e897dcb78 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -2925,6 +2925,7 @@ "follow_cta": "Follow on X" }, "firstrun": { + "backend_stop_failed": "VoiceStudio couldn't safely stop the previous backend. Close any other VoiceStudio window or process, then try again.", "loading": "Preparing setup…", "title": "Set up VoiceStudio", "subtitle": "Nothing's installed yet — review where everything goes, then start. Change it later in Settings.", diff --git a/frontend/src/i18n/locales/es.json b/frontend/src/i18n/locales/es.json index bd40cc66d..40212f609 100644 --- a/frontend/src/i18n/locales/es.json +++ b/frontend/src/i18n/locales/es.json @@ -2198,6 +2198,7 @@ "retry_load": "Reintentar" }, "firstrun": { + "backend_stop_failed": "VoiceStudio no pudo detener de forma segura el backend anterior. Cierra cualquier otra ventana o proceso de VoiceStudio y vuelve a intentarlo.", "loading": "Preparando la configuración…", "title": "Configurar VoiceStudio", "subtitle": "Aún no se ha instalado nada: revisa dónde irá cada cosa y luego comienza. Podrás cambiarlo después en Ajustes.", diff --git a/frontend/src/i18n/locales/fr.json b/frontend/src/i18n/locales/fr.json index 28354923f..5284e7a40 100644 --- a/frontend/src/i18n/locales/fr.json +++ b/frontend/src/i18n/locales/fr.json @@ -2198,6 +2198,7 @@ "retry_load": "Réessayer" }, "firstrun": { + "backend_stop_failed": "VoiceStudio n’a pas pu arrêter l’ancien backend en toute sécurité. Fermez toute autre fenêtre ou tout autre processus VoiceStudio, puis réessayez.", "loading": "Préparation de la configuration…", "title": "Configurer VoiceStudio", "subtitle": "Rien n'est encore installé : vérifiez où tout sera placé, puis lancez. Modifiable plus tard dans les Réglages.", diff --git a/frontend/src/i18n/locales/hi.json b/frontend/src/i18n/locales/hi.json index f8e19f1cb..0329d89b0 100644 --- a/frontend/src/i18n/locales/hi.json +++ b/frontend/src/i18n/locales/hi.json @@ -2198,6 +2198,7 @@ "retry_load": "पुनः प्रयास करें" }, "firstrun": { + "backend_stop_failed": "VoiceStudio पिछले बैकएंड को सुरक्षित रूप से बंद नहीं कर सका। VoiceStudio की कोई अन्य विंडो या प्रक्रिया बंद करें, फिर दोबारा कोशिश करें।", "loading": "सेटअप तैयार हो रहा है…", "title": "VoiceStudio सेट करें", "subtitle": "अभी कुछ भी इंस्टॉल नहीं हुआ है — देखें कि सब कहाँ जाएगा, फिर शुरू करें। बाद में सेटिंग्स में बदल सकते हैं।", diff --git a/frontend/src/i18n/locales/id.json b/frontend/src/i18n/locales/id.json index 8226cebb0..b947531f1 100644 --- a/frontend/src/i18n/locales/id.json +++ b/frontend/src/i18n/locales/id.json @@ -2198,6 +2198,7 @@ "retry_load": "Coba lagi" }, "firstrun": { + "backend_stop_failed": "VoiceStudio tidak dapat menghentikan backend sebelumnya dengan aman. Tutup jendela atau proses VoiceStudio lain, lalu coba lagi.", "loading": "Menyiapkan penyiapan…", "title": "Siapkan VoiceStudio", "subtitle": "Belum ada yang terpasang — tinjau ke mana semuanya disimpan, lalu mulai. Bisa diubah nanti di Pengaturan.", diff --git a/frontend/src/i18n/locales/it.json b/frontend/src/i18n/locales/it.json index b12d09055..3b7a51c84 100644 --- a/frontend/src/i18n/locales/it.json +++ b/frontend/src/i18n/locales/it.json @@ -2198,6 +2198,7 @@ "retry_load": "Riprova" }, "firstrun": { + "backend_stop_failed": "VoiceStudio non è riuscito ad arrestare in sicurezza il backend precedente. Chiudi ogni altra finestra o processo di VoiceStudio e riprova.", "loading": "Preparazione della configurazione…", "title": "Configura VoiceStudio", "subtitle": "Non è stato ancora installato nulla: controlla dove andrà tutto, poi avvia. Potrai cambiarlo nelle Impostazioni.", diff --git a/frontend/src/i18n/locales/ja.json b/frontend/src/i18n/locales/ja.json index d005482b2..0cd21c8bd 100644 --- a/frontend/src/i18n/locales/ja.json +++ b/frontend/src/i18n/locales/ja.json @@ -2198,6 +2198,7 @@ "retry_load": "再試行" }, "firstrun": { + "backend_stop_failed": "VoiceStudio は以前のバックエンドを安全に停止できませんでした。他の VoiceStudio のウィンドウまたはプロセスを閉じて、もう一度お試しください。", "loading": "セットアップを準備中…", "title": "VoiceStudio のセットアップ", "subtitle": "まだ何もインストールされていません。保存先を確認してから開始してください。後で設定から変更できます。", diff --git a/frontend/src/i18n/locales/ko.json b/frontend/src/i18n/locales/ko.json index b49024673..41460db80 100644 --- a/frontend/src/i18n/locales/ko.json +++ b/frontend/src/i18n/locales/ko.json @@ -2198,6 +2198,7 @@ "retry_load": "재시도" }, "firstrun": { + "backend_stop_failed": "VoiceStudio가 이전 백엔드를 안전하게 중지하지 못했습니다. 다른 VoiceStudio 창이나 프로세스를 닫은 후 다시 시도하세요.", "loading": "설정 준비 중…", "title": "VoiceStudio 설정", "subtitle": "아직 아무것도 설치되지 않았습니다. 저장 위치를 확인한 뒤 시작하세요. 나중에 설정에서 변경할 수 있습니다.", diff --git a/frontend/src/i18n/locales/nl.json b/frontend/src/i18n/locales/nl.json index 0eeb211e7..0a63138d4 100644 --- a/frontend/src/i18n/locales/nl.json +++ b/frontend/src/i18n/locales/nl.json @@ -2198,6 +2198,7 @@ "retry_load": "Opnieuw proberen" }, "firstrun": { + "backend_stop_failed": "VoiceStudio kon de vorige backend niet veilig stoppen. Sluit alle andere VoiceStudio-vensters of -processen en probeer het opnieuw.", "loading": "Installatie voorbereiden…", "title": "VoiceStudio instellen", "subtitle": "Er is nog niets geïnstalleerd — controleer waar alles komt te staan en start daarna. Later te wijzigen in Instellingen.", diff --git a/frontend/src/i18n/locales/pl.json b/frontend/src/i18n/locales/pl.json index ee1f2fd17..4add37eb2 100644 --- a/frontend/src/i18n/locales/pl.json +++ b/frontend/src/i18n/locales/pl.json @@ -2198,6 +2198,7 @@ "retry_load": "Spróbuj ponownie" }, "firstrun": { + "backend_stop_failed": "VoiceStudio nie mógł bezpiecznie zatrzymać poprzedniego backendu. Zamknij wszystkie inne okna lub procesy VoiceStudio i spróbuj ponownie.", "loading": "Przygotowywanie konfiguracji…", "title": "Skonfiguruj VoiceStudio", "subtitle": "Nic nie zostało jeszcze zainstalowane — sprawdź, gdzie wszystko trafi, a potem rozpocznij. Można to później zmienić w Ustawieniach.", diff --git a/frontend/src/i18n/locales/pt.json b/frontend/src/i18n/locales/pt.json index 687ef21e6..70f78fdaa 100644 --- a/frontend/src/i18n/locales/pt.json +++ b/frontend/src/i18n/locales/pt.json @@ -2198,6 +2198,7 @@ "retry_load": "Tentar novamente" }, "firstrun": { + "backend_stop_failed": "O VoiceStudio não conseguiu parar o backend anterior em segurança. Feche qualquer outra janela ou processo do VoiceStudio e tente novamente.", "loading": "Preparando a configuração…", "title": "Configurar o VoiceStudio", "subtitle": "Nada foi instalado ainda — confira onde tudo será salvo e então comece. Você pode mudar depois nas Configurações.", diff --git a/frontend/src/i18n/locales/ru.json b/frontend/src/i18n/locales/ru.json index 73a67b57a..3b085e321 100644 --- a/frontend/src/i18n/locales/ru.json +++ b/frontend/src/i18n/locales/ru.json @@ -2198,6 +2198,7 @@ "retry_load": "Повторить попытку" }, "firstrun": { + "backend_stop_failed": "VoiceStudio не смог безопасно остановить предыдущий бэкенд. Закройте другие окна или процессы VoiceStudio и повторите попытку.", "loading": "Подготовка установки…", "title": "Настройка VoiceStudio", "subtitle": "Пока ничего не установлено — проверьте, куда всё будет сохранено, затем начните. Это можно изменить позже в настройках.", diff --git a/frontend/src/i18n/locales/sv.json b/frontend/src/i18n/locales/sv.json index 5288bb390..ca23b5fdd 100644 --- a/frontend/src/i18n/locales/sv.json +++ b/frontend/src/i18n/locales/sv.json @@ -2198,6 +2198,7 @@ "retry_load": "Försök igen" }, "firstrun": { + "backend_stop_failed": "VoiceStudio kunde inte stoppa den tidigare backend-processen säkert. Stäng alla andra VoiceStudio-fönster eller -processer och försök igen.", "loading": "Förbereder installationen…", "title": "Konfigurera VoiceStudio", "subtitle": "Inget är installerat ännu — granska var allt hamnar och starta sedan. Kan ändras senare i Inställningar.", diff --git a/frontend/src/i18n/locales/th.json b/frontend/src/i18n/locales/th.json index 819d5e588..a321c142d 100644 --- a/frontend/src/i18n/locales/th.json +++ b/frontend/src/i18n/locales/th.json @@ -2198,6 +2198,7 @@ "retry_load": "ลองอีกครั้ง" }, "firstrun": { + "backend_stop_failed": "VoiceStudio ไม่สามารถหยุดแบ็กเอนด์ก่อนหน้าได้อย่างปลอดภัย โปรดปิดหน้าต่างหรือกระบวนการ VoiceStudio อื่น แล้วลองอีกครั้ง", "loading": "กำลังเตรียมการติดตั้ง…", "title": "ตั้งค่า VoiceStudio", "subtitle": "ยังไม่มีการติดตั้งใด ๆ — ตรวจสอบตำแหน่งจัดเก็บทั้งหมดก่อน แล้วจึงเริ่ม สามารถเปลี่ยนภายหลังได้ในการตั้งค่า", diff --git a/frontend/src/i18n/locales/tr.json b/frontend/src/i18n/locales/tr.json index 7a5714d5d..2280be4fa 100644 --- a/frontend/src/i18n/locales/tr.json +++ b/frontend/src/i18n/locales/tr.json @@ -2198,6 +2198,7 @@ "retry_load": "Yeniden dene" }, "firstrun": { + "backend_stop_failed": "VoiceStudio önceki arka ucu güvenli biçimde durduramadı. Diğer VoiceStudio pencerelerini veya işlemlerini kapatıp yeniden deneyin.", "loading": "Kurulum hazırlanıyor…", "title": "VoiceStudio'yu kur", "subtitle": "Henüz hiçbir şey kurulmadı — her şeyin nereye gideceğini gözden geçirin, sonra başlatın. Daha sonra Ayarlar'dan değiştirebilirsiniz.", diff --git a/frontend/src/i18n/locales/uk.json b/frontend/src/i18n/locales/uk.json index 84a6e3341..1dd188251 100644 --- a/frontend/src/i18n/locales/uk.json +++ b/frontend/src/i18n/locales/uk.json @@ -2198,6 +2198,7 @@ "retry_load": "Повторіть спробу" }, "firstrun": { + "backend_stop_failed": "VoiceStudio не вдалося безпечно зупинити попередній бекенд. Закрийте інші вікна або процеси VoiceStudio та повторіть спробу.", "loading": "Підготовка налаштування…", "title": "Налаштування VoiceStudio", "subtitle": "Поки нічого не встановлено — перегляньте, куди все буде збережено, і починайте. Потім це можна змінити в Налаштуваннях.", diff --git a/frontend/src/i18n/locales/vi.json b/frontend/src/i18n/locales/vi.json index f0a3c9afa..44079a513 100644 --- a/frontend/src/i18n/locales/vi.json +++ b/frontend/src/i18n/locales/vi.json @@ -2198,6 +2198,7 @@ "retry_load": "Thử lại" }, "firstrun": { + "backend_stop_failed": "VoiceStudio không thể dừng backend trước đó một cách an toàn. Hãy đóng mọi cửa sổ hoặc tiến trình VoiceStudio khác rồi thử lại.", "loading": "Đang chuẩn bị thiết lập…", "title": "Thiết lập VoiceStudio", "subtitle": "Chưa có gì được cài đặt — hãy xem lại nơi lưu mọi thứ rồi bắt đầu. Có thể thay đổi sau trong Cài đặt.", diff --git a/frontend/src/i18n/locales/zh-CN.json b/frontend/src/i18n/locales/zh-CN.json index cf6b88456..790e2d183 100644 --- a/frontend/src/i18n/locales/zh-CN.json +++ b/frontend/src/i18n/locales/zh-CN.json @@ -2205,6 +2205,7 @@ "retry_load": "重试" }, "firstrun": { + "backend_stop_failed": "VoiceStudio 无法安全停止先前的后端。请关闭其他 VoiceStudio 窗口或进程，然后重试。", "loading": "正在准备安装向导…", "title": "设置 VoiceStudio", "subtitle": "尚未安装任何内容——先确认各项存储位置，再开始安装。之后可在设置中更改。", diff --git a/frontend/src/i18n/locales/zh-TW.json b/frontend/src/i18n/locales/zh-TW.json index 9266f108b..cc5adea37 100644 --- a/frontend/src/i18n/locales/zh-TW.json +++ b/frontend/src/i18n/locales/zh-TW.json @@ -2198,6 +2198,7 @@ "retry_load": "重試" }, "firstrun": { + "backend_stop_failed": "VoiceStudio 無法安全停止先前的後端。請關閉其他 VoiceStudio 視窗或程序，然後重試。", "loading": "正在準備安裝精靈…", "title": "設定 VoiceStudio", "subtitle": "尚未安裝任何內容——先確認各項儲存位置，再開始安裝。之後可在設定中變更。", diff --git a/frontend/src/test/FirstRunSetupPortableDir.test.jsx b/frontend/src/test/FirstRunSetupPortableDir.test.jsx index e66bd0e2c..d28d237de 100644 --- a/frontend/src/test/FirstRunSetupPortableDir.test.jsx +++ b/frontend/src/test/FirstRunSetupPortableDir.test.jsx @@ -74,7 +74,8 @@ const renderSetup = () => , ); -beforeEach(() => { +beforeEach(async () => { + await i18n.changeLanguage('en'); invokeMock.mockReset(); openMock.mockReset(); }); @@ -127,6 +128,35 @@ describe('FirstRunSetup — portable folder is choosable', () => { }); }); + it('localizes a lifecycle stop failure instead of exposing the Rust error code', async () => { + const state = setupState(); + invokeMock.mockImplementation(async (cmd) => { + if (cmd === 'get_setup_state') return state; + if (cmd === 'check_install_target') return { writable: true, freeBytes: 500e9 }; + if (cmd === 'complete_setup') throw new Error('backend_stop_failed'); + return undefined; + }); + renderSetup(); + + const startBtn = () => + screen + .getAllByRole('button') + .find((button) => /start installation/i.test(button.textContent || '')); + await waitFor( + () => { + expect(startBtn()).toBeTruthy(); + expect(startBtn().disabled).toBe(false); + }, + { timeout: 4000 }, + ); + await act(async () => { + startBtn().click(); + }); + + expect(await screen.findByText(/couldn't safely stop the previous backend/i)).toBeTruthy(); + expect(screen.queryByText('backend_stop_failed')).toBeNull(); + }); + it('promises portability only for a folder INSIDE the app directory', async () => { // Only that case can be stored as a relative path, which is what survives // the mount path changing (/Volumes/Stick on one machine, E:\\ on the next). diff --git a/tests/test_sidecar_install.py b/tests/test_sidecar_install.py index 2d8148cd6..c438aa705 100644 --- a/tests/test_sidecar_install.py +++ b/tests/test_sidecar_install.py @@ -9,8 +9,11 @@ """ import io import os +import subprocess +import sys import tarfile import threading +import time from pathlib import Path from types import SimpleNamespace @@ -45,6 +48,7 @@ def _clean_state(monkeypatch, tmp_path): monkeypatch.setattr(si, "_jobs", {}) monkeypatch.delenv("OMNIVOICE_INDEXTTS_DIR", raising=False) monkeypatch.delenv("OMNIVOICE_FAKE_SIDE_DIR", raising=False) + monkeypatch.delenv("OMNIVOICE_DESKTOP_CONTAINED", raising=False) yield @@ -281,10 +285,7 @@ def test_git_failure_falls_back_to_tarball(monkeypatch): assert (si.managed_checkout(spec) / "pyproject.toml").is_file() -def test_kill_tree_uses_taskkill_on_windows(monkeypatch): - """On Windows proc.kill() fells only the direct child — a spawned git/uv - helper would keep writing into the checkout past the timeout. The tree - kill must go through taskkill /T there (POSIX uses killpg).""" +def test_legacy_kill_fallback_uses_only_direct_stable_handle(monkeypatch): calls = {} monkeypatch.setattr(si.os, "name", "nt") monkeypatch.setattr( @@ -293,9 +294,95 @@ def test_kill_tree_uses_taskkill_on_windows(monkeypatch): ) proc = SimpleNamespace(pid=4242, kill=lambda: calls.setdefault("plain_kill", True)) si._kill_tree(proc) - assert calls["argv"][:4] == ["taskkill", "/F", "/T", "/PID"] - assert calls["argv"][4] == "4242" - assert "plain_kill" not in calls # taskkill succeeded — no fallback + assert calls == {"plain_kill": True} + + +def test_desktop_installer_needs_no_unmanaged_spawn_flags(monkeypatch): + monkeypatch.setenv("OMNIVOICE_DESKTOP_CONTAINED", "1") + monkeypatch.setattr(si.os, "name", "posix") + assert si._install_containment_kwargs() == {} + + +def test_standalone_installer_also_delegates_to_nested_owner(monkeypatch): + monkeypatch.delenv("OMNIVOICE_DESKTOP_CONTAINED", raising=False) + monkeypatch.setattr(si.os, "name", "posix") + assert si._install_containment_kwargs() == {} + + +def test_desktop_windows_timeout_never_taskkills_a_reusable_pid(monkeypatch): + calls = {} + monkeypatch.setenv("OMNIVOICE_DESKTOP_CONTAINED", "1") + monkeypatch.setattr(si.os, "name", "nt") + monkeypatch.setattr( + si.subprocess, + "run", + lambda *args, **kwargs: calls.setdefault("taskkill", True), + ) + proc = SimpleNamespace(pid=4242, kill=lambda: calls.setdefault("handle_kill", True)) + si._kill_tree(proc) + assert calls == {"handle_kill": True} + + +def test_desktop_installer_timeout_kills_nested_helper_before_it_can_mutate( + monkeypatch, tmp_path +): + """A timed-out uv/git root must not leave its pipe-owning helpers alive.""" + marker = tmp_path / "late-mutation" + monkeypatch.setenv("OMNIVOICE_DESKTOP_CONTAINED", "1") + drain_read, drain_write = os.pipe() + monkeypatch.setenv("OMNIVOICE_DESKTOP_DRAIN_FD", str(drain_write)) + child_script = ( + "import os,time; time.sleep(1); " + "open(os.environ['OMNIVOICE_TIMEOUT_MARKER'], 'w').write('bad')" + ) + script = ( + "import subprocess,sys,time; " + f"subprocess.Popen([sys.executable, '-c', {child_script!r}]); " + "time.sleep(60)" + ) + env = os.environ.copy() + env["OMNIVOICE_TIMEOUT_MARKER"] = str(marker) + job = si._new_job("fake-side") + + try: + assert si._run_logged(job, [sys.executable, "-c", script], timeout=0.2, env=env) == -1 + time.sleep(1.2) + assert not marker.exists() + finally: + os.close(drain_write) + os.close(drain_read) + + +def test_backend_death_closes_control_pipe_and_kills_nested_operation(tmp_path): + """Outer desktop teardown reaches an operation which owns a nested group.""" + marker = tmp_path / "mutation-after-backend-death" + operation = ( + "import os,time; time.sleep(1); " + "open(os.environ['OMNIVOICE_TIMEOUT_MARKER'], 'w').write('bad')" + ) + backend = ( + "import os,sys,time; " + "from core.contained_subprocess import spawn_owned; " + f"spawn_owned([sys.executable, '-c', {operation!r}]); " + "time.sleep(.2); os._exit(0)" + ) + env = os.environ.copy() + env["OMNIVOICE_DESKTOP_CONTAINED"] = "1" + env["OMNIVOICE_TIMEOUT_MARKER"] = str(marker) + env["PYTHONPATH"] = str(Path(__file__).parents[1] / "backend") + + run_kwargs = {} + drain_fds = None + if os.name == "posix": + drain_fds = os.pipe() + env["OMNIVOICE_DESKTOP_DRAIN_FD"] = str(drain_fds[1]) + run_kwargs["pass_fds"] = (drain_fds[1],) + assert subprocess.run([sys.executable, "-c", backend], env=env, **run_kwargs).returncode == 0 + if drain_fds is not None: + os.close(drain_fds[1]) + os.close(drain_fds[0]) + time.sleep(1.2) + assert not marker.exists() def test_safe_extract_members_blocks_tar_slip(tmp_path): diff --git a/tests/test_worker_inbound_transport.py b/tests/test_worker_inbound_transport.py index fd9373e02..bcf90e466 100644 --- a/tests/test_worker_inbound_transport.py +++ b/tests/test_worker_inbound_transport.py @@ -198,15 +198,17 @@ async def connect_panel(self, secret=None, *, wait=True): self.connection = self.worker.NodeConnection(self.servicer, connection) self.connector_task = asyncio.create_task(self.connection.run_forever()) if wait: - def worker_is_ready(): + def worker_is_activated(): if len(self.pool) != 1: return False live = next(iter(self.pool)) - return live.record.schedulable and live.supports( - engine=ENGINE, model_id=MODEL, operation=OP + return ( + live.record.schedulable + and not live.registration_pending + and live.supports(engine=ENGINE, model_id=MODEL, operation=OP) ) - await _until(worker_is_ready) + await _until(worker_is_activated) return self.connection async def stop(self): @@ -252,6 +254,8 @@ async def test_a_panel_that_dials_a_node_ends_up_with_a_schedulable_worker(inbou assert len(inbound.pool) == 1 worker = next(iter(inbound.pool)) assert worker.record.schedulable is True + assert worker.registration_pending is False + assert worker.capacity.can_accept(ENGINE, MODEL) # Capabilities crossed the inverted stream, so the scheduler can actually # pick this worker rather than merely knowing it exists. assert worker.supports(engine=ENGINE, model_id=MODEL, operation=OP)