fix(update): stop the venv-side launcher of each paused Windows gateway
On Windows a gateway started through the venv shim is a two-process chain:
venv\Scripts\python.exe (launcher — keeps venv .pyd files mapped)
└─ uv\python\...\python.exe (worker — writes the gateway PID file)
`_pause_windows_gateways_for_update()` builds its pause set from
`find_gateway_pids()`, which reads the PID file and therefore only ever
sees the *worker*. The venv-holder guard immediately downstream
(`_detect_venv_python_processes()`) matches on the venv path prefix, so it
only ever sees the *launcher*.
The two sets are disjoint. A gateway the updater had just gracefully
drained still left its launcher alive, the guard reported that launcher as
a venv holder, and the update aborted — every time. On the Desktop path
this surfaces as the dead-end dialog:
[updates] venv-blocked: 2 process(es) hold the install
PID ... python.exe ...\venv\Scripts\python.exe -m hermes_cli.main gateway run --replace
Note the reported holder is a gateway the updater believes it stopped.
The Desktop path is affected because `hermes-setup.exe` runs
`hermes update --yes --gateway --force`, and `--force` deliberately does
NOT bypass the venv guard (that needs `--force-venv`), so the abort is
correct behaviour reacting to an incomplete pause.
Fix: after the graceful drain, walk one hop up from each mapped gateway
PID and force-kill parents that live under the project venv.
Deliberately additive, not a substitution:
- The planned-stop marker and the graceful drain still target the worker
(the PID that wrote the PID file), so clean shutdown is unchanged and
updates don't get pushed onto the hard-kill path.
- `terminate_pid(force=True)` is `taskkill /T` (tree kill), so killing a
launcher that outlived its worker also reaps stragglers.
- `_resume_windows_gateways_after_update()` needs no change: the mapped
respawn argv is rebuilt from the profile name
(`_gateway_run_args_for_profile`), never from the killed PID, and the
restart watcher's `_pid_exists()` wait still terminates because the
tree kill takes the whole chain down.
- Only the venv-side parent is returned. Unrelated ancestors (a Scheduled
Task's `cmd.exe`, an operator's shell) are ignored, and the caller's own
process chain is excluded so a CLI `hermes update` never nominates
itself.
Tests assert the invariant the two PID-resolution paths must satisfy —
the pause's kill set must cover the guard's abort set — rather than
snapshotting PIDs. Verified to fail without the fix:
AssertionError: pause stopped [] but the venv guard aborts on [400]
— disjoint sets abort the update
This commit is contained in:
parent
f9522fdcef
commit
9507f4382e
|
|
@ -5071,6 +5071,7 @@ from hermes_cli.update_cmd import ( # noqa: F401
|
|||
_upgrade_pip_before_lazy_refresh,
|
||||
_validate_critical_files_syntax,
|
||||
_venv_core_imports_healthy,
|
||||
_venv_launcher_ancestors,
|
||||
_wait_for_windows_update_gateway_exit,
|
||||
_warn_incomplete_gateway_fleet_restart,
|
||||
_web_build_toolchain_ready,
|
||||
|
|
|
|||
|
|
@ -2683,6 +2683,72 @@ def _format_venv_python_holders_message(matches: list[tuple[int, str, str]]) ->
|
|||
lines.append(" (or use `hermes update --force-venv` to proceed anyway at your own risk)")
|
||||
return "\n".join(lines)
|
||||
|
||||
def _venv_launcher_ancestors(pids: list[int]) -> list[int]:
|
||||
"""Return venv-interpreter ancestors of *pids* that hold the install open.
|
||||
|
||||
On Windows a gateway started through the venv shim is a **two-process
|
||||
chain**: ``venv\\Scripts\\python.exe`` (the launcher, which keeps native
|
||||
``.pyd`` files from the venv mapped) spawns the actual interpreter from
|
||||
uv's managed CPython directory (``AppData\\Roaming\\uv\\python\\...``).
|
||||
The gateway writes its PID file from the *child*, so
|
||||
``find_gateway_pids()`` — and therefore this module's pause set — only
|
||||
ever sees the uv-side worker.
|
||||
|
||||
``_detect_venv_python_processes()`` matches on the venv path prefix, so
|
||||
the guard downstream of the pause sees the *launcher* instead. The two
|
||||
sets are disjoint, which meant a paused gateway still tripped the
|
||||
venv-holder guard and aborted the update every time (the Desktop
|
||||
"venv-blocked: N process(es) hold the install" dead-end, where the
|
||||
reported holder is a gateway the updater believes it already stopped).
|
||||
|
||||
Walking one hop up from each mapped gateway PID and keeping ancestors
|
||||
that live under the project venv closes the gap. Only the venv-side
|
||||
parent is returned — unrelated ancestors (the Scheduled Task's
|
||||
``cmd.exe``, an operator's shell) are ignored so we never widen the
|
||||
blast radius beyond the gateway's own launcher. Never raises.
|
||||
"""
|
||||
if not _m()._is_windows() or not pids:
|
||||
return []
|
||||
try:
|
||||
import psutil
|
||||
except Exception:
|
||||
return []
|
||||
|
||||
venv_dir = _m().PROJECT_ROOT / "venv"
|
||||
try:
|
||||
venv_prefix = str(venv_dir.resolve()).lower().rstrip(os.sep) + os.sep
|
||||
except OSError:
|
||||
venv_prefix = str(venv_dir).lower().rstrip(os.sep) + os.sep
|
||||
|
||||
# Never return ourselves or our own ancestry: a CLI ``hermes update``
|
||||
# runs from the venv python and would otherwise nominate itself.
|
||||
skip: set[int] = {os.getpid()}
|
||||
try:
|
||||
for anc in psutil.Process().parents():
|
||||
skip.add(int(anc.pid))
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
found: list[int] = []
|
||||
for pid in pids:
|
||||
try:
|
||||
parent = psutil.Process(int(pid)).parent()
|
||||
except Exception:
|
||||
continue
|
||||
if parent is None:
|
||||
continue
|
||||
ppid = int(parent.pid)
|
||||
if ppid in skip or ppid in found or ppid in set(pids):
|
||||
continue
|
||||
try:
|
||||
exe = (parent.exe() or "").lower()
|
||||
except Exception:
|
||||
continue
|
||||
if exe.startswith(venv_prefix):
|
||||
found.append(ppid)
|
||||
return found
|
||||
|
||||
|
||||
def _pause_windows_gateways_for_update() -> dict | None:
|
||||
"""Stop running Windows gateways before mutating the checkout or venv.
|
||||
|
||||
|
|
@ -2783,8 +2849,18 @@ def _pause_windows_gateways_for_update() -> dict | None:
|
|||
logger.debug("Could not capture argv for unmapped gateway %s: %s", pid, exc)
|
||||
unmapped.append({"pid": int(pid), "argv": argv})
|
||||
|
||||
# A gateway's graceful drain above targets the PID that wrote the PID file
|
||||
# (the uv-side worker). On Windows that worker's parent is usually the
|
||||
# venv-side ``python.exe`` launcher, which keeps venv ``.pyd`` files mapped
|
||||
# and is what ``_detect_venv_python_processes()`` reports downstream. Left
|
||||
# alive, it trips the venv-holder guard and aborts the update even though
|
||||
# the gateway itself is stopped. Force-kill those launchers alongside the
|
||||
# survivors; ``terminate_pid(force=True)`` is a tree kill, so a launcher
|
||||
# that outlived its worker takes any stragglers with it.
|
||||
launcher_pids = _m()._venv_launcher_ancestors(mapped_pids)
|
||||
|
||||
force_killed = []
|
||||
for pid in sorted(set(survivors).union(unmapped_pids)):
|
||||
for pid in sorted(set(survivors).union(unmapped_pids).union(launcher_pids)):
|
||||
try:
|
||||
terminate_pid(int(pid), force=True)
|
||||
force_killed.append(int(pid))
|
||||
|
|
|
|||
|
|
@ -270,6 +270,138 @@ def test_pause_windows_gateways_for_update_stops_profile_and_unmapped_pids(
|
|||
assert "Restart manually after update" not in captured
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# venv-side launcher ancestors (the uv launcher/worker split)
|
||||
#
|
||||
# A gateway started through the venv shim is two processes:
|
||||
# venv\Scripts\python.exe (launcher) -> uv\python\...\python.exe (worker)
|
||||
# The gateway's PID file records the WORKER, so find_gateway_pids() (and the
|
||||
# pause set built from it) only ever sees the worker. The venv-holder guard
|
||||
# matches on the venv path prefix, so it only ever sees the LAUNCHER. The two
|
||||
# sets were disjoint: a gateway the updater had just stopped still tripped the
|
||||
# guard, aborting every update ("venv-blocked: N process(es) hold the install").
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _fake_psutil_tree(tree, venv_exe, worker_exe):
|
||||
"""Build a psutil stand-in where ``tree`` maps worker pid -> parent pid.
|
||||
|
||||
Parents whose pid is even are venv-side (``venv_exe``); odd parents are
|
||||
unrelated ancestors (``worker_exe``) that must NOT be returned.
|
||||
"""
|
||||
|
||||
class FakeProc:
|
||||
def __init__(self, pid):
|
||||
self.pid = pid
|
||||
if pid not in tree and pid not in tree.values():
|
||||
raise ValueError(f"no such pid {pid}")
|
||||
|
||||
def parent(self):
|
||||
ppid = tree.get(self.pid)
|
||||
return FakeProc(ppid) if ppid else None
|
||||
|
||||
def parents(self):
|
||||
return []
|
||||
|
||||
def exe(self):
|
||||
# Parents of workers are the launchers under test.
|
||||
return venv_exe if self.pid % 2 == 0 else worker_exe
|
||||
|
||||
mod = types.SimpleNamespace(Process=FakeProc)
|
||||
return mod
|
||||
|
||||
|
||||
@patch.object(cli_main, "_is_windows", return_value=True)
|
||||
def test_venv_launcher_ancestors_returns_venv_side_parent(_winp, monkeypatch):
|
||||
"""The worker's venv-side parent is reported so the guard set is covered."""
|
||||
venv_exe = str(cli_main.PROJECT_ROOT / "venv" / "Scripts" / "python.exe")
|
||||
worker_exe = r"C:\Users\x\AppData\Roaming\uv\python\cpython-3.11\python.exe"
|
||||
|
||||
# worker 200 -> launcher 100 (even == venv-side)
|
||||
fake = _fake_psutil_tree({200: 100}, venv_exe, worker_exe)
|
||||
monkeypatch.setitem(sys.modules, "psutil", fake)
|
||||
|
||||
assert cli_main._venv_launcher_ancestors([200]) == [100]
|
||||
|
||||
|
||||
@patch.object(cli_main, "_is_windows", return_value=True)
|
||||
def test_venv_launcher_ancestors_ignores_non_venv_parents(_winp, monkeypatch):
|
||||
"""A Scheduled Task's cmd.exe / an operator shell is not a venv holder."""
|
||||
venv_exe = str(cli_main.PROJECT_ROOT / "venv" / "Scripts" / "python.exe")
|
||||
worker_exe = r"C:\Windows\System32\cmd.exe"
|
||||
|
||||
# worker 200 -> parent 101 (odd == NOT venv-side)
|
||||
fake = _fake_psutil_tree({200: 101}, venv_exe, worker_exe)
|
||||
monkeypatch.setitem(sys.modules, "psutil", fake)
|
||||
|
||||
assert cli_main._venv_launcher_ancestors([200]) == []
|
||||
|
||||
|
||||
@patch.object(cli_main, "_is_windows", return_value=True)
|
||||
def test_venv_launcher_ancestors_is_empty_without_pids(_winp):
|
||||
"""No mapped gateways means nothing to walk up from."""
|
||||
assert cli_main._venv_launcher_ancestors([]) == []
|
||||
|
||||
|
||||
@patch.object(cli_main, "_is_windows", return_value=True)
|
||||
def test_pause_kill_set_covers_venv_guard_abort_set(
|
||||
_winp,
|
||||
monkeypatch,
|
||||
tmp_path,
|
||||
):
|
||||
"""INVARIANT: whatever the venv guard would abort on must be stopped.
|
||||
|
||||
This is the contract the two PID-resolution paths must satisfy. Before the
|
||||
launcher walk existed, ``terminated`` held only the uv-side worker while
|
||||
the guard reported the venv-side launcher, so the update aborted forever
|
||||
despite a "successful" pause.
|
||||
"""
|
||||
import hermes_cli.gateway as gateway_mod
|
||||
import gateway.status as status_mod
|
||||
|
||||
venv_exe = str(cli_main.PROJECT_ROOT / "venv" / "Scripts" / "python.exe")
|
||||
worker_exe = r"C:\Users\x\AppData\Roaming\uv\python\cpython-3.11\python.exe"
|
||||
|
||||
profile_home = tmp_path / "profiles" / "default"
|
||||
profile_home.mkdir(parents=True)
|
||||
# The PID file records the WORKER (even-numbered parent 400 is its launcher).
|
||||
worker_pid, launcher_pid = 500, 400
|
||||
profile_proc = SimpleNamespace(
|
||||
profile="default", path=profile_home, pid=worker_pid
|
||||
)
|
||||
|
||||
monkeypatch.setattr(gateway_mod, "find_gateway_pids", lambda **_k: [worker_pid])
|
||||
monkeypatch.setattr(
|
||||
gateway_mod, "find_profile_gateway_processes", lambda **_k: [profile_proc]
|
||||
)
|
||||
monkeypatch.setattr(gateway_mod, "_get_restart_drain_timeout", lambda: 0.1)
|
||||
# Graceful drain succeeds: the worker exits, leaving zero survivors. This is
|
||||
# precisely the case that used to leave the launcher alive and abort.
|
||||
monkeypatch.setattr(
|
||||
cli_main, "_wait_for_windows_update_gateway_exit", lambda pids, *, timeout: set()
|
||||
)
|
||||
|
||||
fake = _fake_psutil_tree({worker_pid: launcher_pid}, venv_exe, worker_exe)
|
||||
monkeypatch.setitem(sys.modules, "psutil", fake)
|
||||
|
||||
terminated = []
|
||||
monkeypatch.setattr(
|
||||
status_mod,
|
||||
"terminate_pid",
|
||||
lambda pid, force=False: terminated.append(int(pid)),
|
||||
)
|
||||
|
||||
cli_main._pause_windows_gateways_for_update()
|
||||
|
||||
# What the downstream venv-holder guard would report as blocking.
|
||||
guard_would_abort_on = {launcher_pid}
|
||||
assert guard_would_abort_on.issubset(set(terminated)), (
|
||||
f"pause stopped {sorted(terminated)} but the venv guard aborts on "
|
||||
f"{sorted(guard_would_abort_on)} — disjoint sets abort the update"
|
||||
)
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue