diff --git a/gateway/status.py b/gateway/status.py index af44d035fb..4b30bf8d03 100644 --- a/gateway/status.py +++ b/gateway/status.py @@ -22,6 +22,9 @@ from pathlib import Path from typing import Any, Callable, NamedTuple, Optional from hermes_constants import _get_platform_default_hermes_home, get_hermes_home, get_process_hermes_home +from hermes_cli._subprocess_compat import pid_exists_stdlib +from hermes_cli._subprocess_compat import posix_is_zombie as _posix_is_zombie # noqa: F401 - historical name +from hermes_cli._subprocess_compat import win32_pid_exists as _pid_exists_win32_ctypes # noqa: F401 - historical name from utils import atomic_json_write if sys.platform == "win32": @@ -1055,59 +1058,7 @@ def _pid_exists(pid: int) -> bool: return bool(psutil.pid_exists(pid)) except ImportError: pass # Fall through to stdlib fallback. - if _IS_WINDOWS: - return _pid_exists_win32_ctypes(pid) - if _posix_is_zombie(pid): # a zombie still answers os.kill(pid, 0) - return False - try: - os.kill(pid, 0) # windows-footgun: ok — POSIX-only branch (the whole point of _pid_exists) - except PermissionError: - return True # Exists but we can't signal it. - except OSError: # ProcessLookupError included - return False - return True - - -def _posix_is_zombie(pid: int) -> bool: - """Zombie via ``/proc//stat`` field 3, or ``ps -o state=`` without /proc (macOS/BSD).""" - try: - stat_fields = Path(f"/proc/{pid}/stat").read_text(encoding="utf-8").split() - return len(stat_fields) > 2 and stat_fields[2] == "Z" - except FileNotFoundError: - with contextlib.suppress(Exception): - r = subprocess.run( - ["ps", "-o", "state=", "-p", str(pid)], - capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=5, - ) - return r.returncode == 0 and r.stdout.strip().startswith("Z") - except (IndexError, PermissionError, OSError): - pass - return False - - -def _pid_exists_win32_ctypes(pid: int) -> bool: - """psutil-free Windows liveness probe via OpenProcess/WaitForSingleObject.""" - try: - import ctypes - kernel32 = ctypes.windll.kernel32 # type: ignore[attr-defined] - # Pin restypes: default c_int mangles WAIT_* DWORDs into negatives. - kernel32.OpenProcess.restype = ctypes.c_void_p - kernel32.WaitForSingleObject.restype = ctypes.c_uint - kernel32.GetLastError.restype = ctypes.c_uint - PROCESS_QUERY_LIMITED_INFORMATION, SYNCHRONIZE = 0x1000, 0x100000 # SYNCHRONIZE: for Wait* - WAIT_TIMEOUT, ERROR_ACCESS_DENIED = 0x00000102, 5 - handle = kernel32.OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION | SYNCHRONIZE, False, pid) - if not handle: - # ERROR_INVALID_PARAMETER (87): PID definitely gone. ACCESS_DENIED: exists - # but owned by another user/session. Any other error: conservative False. - return kernel32.GetLastError() == ERROR_ACCESS_DENIED - try: - # WAIT_TIMEOUT = still running; anything else = gone. - return kernel32.WaitForSingleObject(handle, 0) == WAIT_TIMEOUT - finally: - kernel32.CloseHandle(handle) - except (OSError, AttributeError): - return False + return pid_exists_stdlib(pid) def _release_file_lock(handle) -> None: diff --git a/hermes_cli/_subprocess_compat.py b/hermes_cli/_subprocess_compat.py index d550c905b4..5f671e8a0c 100644 --- a/hermes_cli/_subprocess_compat.py +++ b/hermes_cli/_subprocess_compat.py @@ -34,6 +34,7 @@ __all__ = [ "NO_DRIVER_DIFF_FLAGS", "NO_LAZY_FETCH_ENV", "pid_is_hermes", + "pid_exists_stdlib", ] # Flags that neutralize *attribute-scoped* diff drivers on any diff-rendering git command. A @@ -465,6 +466,73 @@ def noninteractive_git_env(base: "Mapping[str, str] | None" = None) -> dict[str, return env +def posix_is_zombie(pid: int) -> bool: + """Zombie via ``/proc//stat`` field 3, or ``ps -o state=`` without /proc (macOS/BSD).""" + try: + with open(f"/proc/{pid}/stat", encoding="utf-8") as fh: + stat_fields = fh.read().split() + return len(stat_fields) > 2 and stat_fields[2] == "Z" + except FileNotFoundError: + try: + r = subprocess.run( + ["ps", "-o", "state=", "-p", str(pid)], + capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=5, + ) + return r.returncode == 0 and r.stdout.strip().startswith("Z") + except Exception: + pass + except (IndexError, PermissionError, OSError): + pass + return False + + +def win32_pid_exists(pid: int) -> bool: + """psutil-free Windows liveness probe via OpenProcess/WaitForSingleObject.""" + try: + import ctypes + kernel32 = ctypes.windll.kernel32 # type: ignore[attr-defined] + # Pin restypes: default c_int mangles WAIT_* DWORDs into negatives. + kernel32.OpenProcess.restype = ctypes.c_void_p + kernel32.WaitForSingleObject.restype = ctypes.c_uint + kernel32.GetLastError.restype = ctypes.c_uint + PROCESS_QUERY_LIMITED_INFORMATION, SYNCHRONIZE = 0x1000, 0x100000 # SYNCHRONIZE: for Wait* + WAIT_TIMEOUT, ERROR_ACCESS_DENIED = 0x00000102, 5 + handle = kernel32.OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION | SYNCHRONIZE, False, pid) + if not handle: + # ERROR_INVALID_PARAMETER (87): PID definitely gone. ACCESS_DENIED: exists + # but owned by another user/session. Any other error: conservative False. + return kernel32.GetLastError() == ERROR_ACCESS_DENIED + try: + # WAIT_TIMEOUT = still running; anything else = gone. + return kernel32.WaitForSingleObject(handle, 0) == WAIT_TIMEOUT + finally: + kernel32.CloseHandle(handle) + except (OSError, AttributeError): + return False + + +def pid_exists_stdlib(pid: int) -> bool: + """Stdlib-only "is this PID alive" check that never signals the target (zombies report dead). + + For code that must run without the dependency environment: the detached gateway restart + watcher is started by whatever interpreter the updater runs on (the bare store Python after + the package-manager handoff), so it cannot import ``gateway.status`` (``utils`` pulls in + ``ruamel``). ``gateway.status._pid_exists`` prefers psutil and falls back to this. + """ + pid = int(pid) + if IS_WINDOWS: + return win32_pid_exists(pid) + if posix_is_zombie(pid): # a zombie still answers os.kill(pid, 0) + return False + try: + os.kill(pid, 0) # windows-footgun: ok — POSIX-only branch (Windows returned above) + except PermissionError: + return True # Exists but we can't signal it. + except OSError: # ProcessLookupError included + return False + return True + + def _process_start_time(pid: int) -> int | None: """The repository's stable process-start fingerprint, if available.""" try: diff --git a/hermes_cli/gateway.py b/hermes_cli/gateway.py index 82723f9afc..a86b6ff50e 100644 --- a/hermes_cli/gateway.py +++ b/hermes_cli/gateway.py @@ -1073,8 +1073,11 @@ def _spawn_gateway_restart_watcher(old_pid: int, run_argv: list[str], *, host: b import subprocess import sys import time + # Stdlib-only imports: the watcher runs on the updater's interpreter, which after the + # package-manager handoff is the bare store Python without the dependency environment. from hermes_cli._subprocess_compat import ( - _WINDOWS_GATEWAY_BREAKAWAY_ENV, windows_detach_flags, windows_detach_flags_without_breakaway, + _WINDOWS_GATEWAY_BREAKAWAY_ENV, pid_exists_stdlib, windows_detach_flags, + windows_detach_flags_without_breakaway, ) pid = int(sys.argv[1]) @@ -1084,8 +1087,7 @@ def _spawn_gateway_restart_watcher(old_pid: int, run_argv: list[str], *, host: b deadline = time.monotonic() + {watcher_timeout_literal} while time.monotonic() < deadline: # ``os.kill(pid, 0)`` is not a no-op on Windows — use the cross-platform existence check. - from gateway.status import _pid_exists - if not _pid_exists(pid): + if not pid_exists_stdlib(pid): break time.sleep(0.2) @@ -1095,7 +1097,7 @@ def _spawn_gateway_restart_watcher(old_pid: int, run_argv: list[str], *, host: b _stdio_target = subprocess.DEVNULL _stdio_fh = None try: - from hermes_cli.config import get_hermes_home + from hermes_constants import get_hermes_home from pathlib import Path _log_dir = Path(get_hermes_home()) / "logs" _log_dir.mkdir(parents=True, exist_ok=True) diff --git a/tests/hermes_cli/test_gateway_restart_watcher_bare_python.py b/tests/hermes_cli/test_gateway_restart_watcher_bare_python.py new file mode 100644 index 0000000000..4b0e838a75 --- /dev/null +++ b/tests/hermes_cli/test_gateway_restart_watcher_bare_python.py @@ -0,0 +1,57 @@ +"""The detached gateway restart watcher must survive on an interpreter without the dependency env. + +``hermes update`` finishes on the bare store Python (dependencies come from ``hermes_bootstrap``, +not site-packages) and spawns the watcher as ``sys.executable -c ``. A watcher that imports +a third-party-backed Hermes module (``gateway.status`` -> ``utils`` -> ``ruamel``) dies before it +relaunches the gateway, leaving a manually started gateway down after every update. +""" + +from __future__ import annotations + +import os +import subprocess +import sys +import time +from pathlib import Path + +import pytest + +from hermes_cli import gateway + +REPO = Path(__file__).resolve().parents[2] + +pytestmark = pytest.mark.skipif(sys.platform == "win32", reason="POSIX shell wrapper stands in for the bare interpreter") + + +def _dead_pid() -> int: + proc = subprocess.Popen([sys.executable, "-c", "pass"]) + proc.wait() + return proc.pid + + +def test_restart_watcher_relaunches_from_an_interpreter_without_site_packages(tmp_path, monkeypatch): + # A python that sees the checkout but no third-party packages, like the bare store Python. + bare = tmp_path / "bare-python" + bare.write_text(f'#!/bin/sh\nunset PYTHONPATH\nexec "{sys.executable}" -S "$@"\n', encoding="utf-8") + bare.chmod(0o755) + probe = subprocess.run([str(bare), "-c", "import ruamel.yaml"], cwd=REPO, capture_output=True, text=True) + assert probe.returncode != 0, "premise: the stand-in interpreter must not see site-packages" + + marker = tmp_path / "respawned" + home = tmp_path / "home" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.chdir(REPO) + monkeypatch.setattr(sys, "executable", str(bare)) + relaunch = [str(bare), "-c", f"open({str(marker)!r}, 'w').write('ok')"] + + assert gateway._spawn_gateway_restart_watcher(_dead_pid(), relaunch, host=False) + + deadline = time.monotonic() + 30 + while time.monotonic() < deadline and not marker.exists(): + time.sleep(0.1) + stdio = home / "logs" / "gateway-stdio.log" + assert marker.exists(), ( + "the restart watcher never relaunched the gateway command" + + (f"; stdio log:\n{stdio.read_text(errors='replace')}" if stdio.exists() else "")) + assert os.path.getsize(marker) == 2