fix(gateway): keep the update restart watcher stdlib-only so it survives the bare store Python
After the package-manager handoff, hermes update finishes on the bare store interpreter and spawns the detached restart watcher as sys.executable -c. The watcher imported gateway.status (utils -> hermes_yaml -> ruamel) and hermes_cli.config, died with ModuleNotFoundError before relaunching, and left every manually started gateway down after the update (gated on #124649). Move the stdlib liveness probe (zombie-aware POSIX kill(0), Windows OpenProcess) into hermes_cli._subprocess_compat, have gateway.status's fallback delegate to it, and import only stdlib-backed modules in the watcher. Fixes #124649.
This commit is contained in:
@@ -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/<pid>/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:
|
||||
|
||||
@@ -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/<pid>/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:
|
||||
|
||||
@@ -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)
|
||||
|
||||
57
tests/hermes_cli/test_gateway_restart_watcher_bare_python.py
Normal file
57
tests/hermes_cli/test_gateway_restart_watcher_bare_python.py
Normal file
@@ -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 <watcher>``. 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
|
||||
Reference in New Issue
Block a user