fix(update): reap orphaned Desktop backends + stop managed runtime child trees (salvages #81327) - #82179
Conversation
…e venv-holder guard The GUI-updater handoff race: the Desktop fires SIGTERM + app.quit() and spawns hermes-setup, but its Python backend (`python.exe -m hermes_cli.main serve`) can survive the teardown. The Desktop is gone -- nothing will respawn that backend -- yet the venv-holder guard refused on it and the update dead-ended with "Hermes is still running" while the user had zero windows open (observed twice on 2026-08-09, 01:59 and 02:17, bootstrap-installer.log). New `_orphaned_desktop_backend_pids()` classifies remaining holders: a serve/dashboard backend whose supervising parent is provably dead (PID gone, or recycled -- parent created after the child) is a straggler safe to reap. Any live-parent backend, non-backend holder, or unprovable case keeps the refusal exactly as before. Reaping uses the new `_stop_process_trees()` (taskkill /T /F), mirroring the Desktop's forceKillProcessTree and install.ps1's venv sweep so the managed .hermes-runtime interpreter child dies with its launcher (#70026). Builds on #81327 (salvaged intact underneath): that fixed the same parent-only-kill gap in install.ps1's venv sweep; this closes the remaining dead-end in the `hermes update` guard itself. E2E on a real Windows box: spawned a detached orphan with a backend-shaped argv -> classifier returned its PID and the tree reap killed it; a non-backend orphan and the live Desktop backend (parent alive) both returned None (refusal preserved).
૮ >ﻌ< ა ci reviewran on bfc2a55
|
|
Thanks for consolidating this and preserving the #81327 commit/authorship. The installer-side tree kill and the orphan recovery both make sense. I think we should also bring over the Desktop teardown portion of #77436 before merging this. Could we also add coverage for a holder set containing both an orphaned My preference would be:
With that, this should cleanly supersede both #81327 and the relevant part of #77436. |
|
Good catch — all three points addressed in #82191 (raised as a follow-up since this merged while your comment landed):
E2E on a real Windows box: mixed orphan tree (root + 3 descendants) classified to |
…te trees without pre-signalling Follow-up to #82179 addressing helix4u's review comment (#82179 issuecomment-5229441571). Three parts: 1. Desktop teardown (salvaged from #77436, @4adwentures): the update hand-off's releaseBackendLock() sent SIGTERM to the primary backend BEFORE taskkill /T. If the launcher exits first, Windows can no longer enumerate its descendants and they survive holding the venv — the Electron path that creates the orphan #82179 then has to repair. New stopBackendTreesForUpdate() tree-kills the live root first, with the behavioral vitest from #77436. The scanner half of #77436 is deliberately NOT taken (superseded by #82158's full-cmdline scan). 2. Tree-aware orphan classification: _orphaned_desktop_backend_pids() previously refused the whole holder set when any holder had a live parent. But the scanner legitimately returns an orphaned serve root AND its descendants (the venv trampoline's uv-managed interpreter worker — which carries the same backend argv — plus .hermes-runtime children). Those have a live parent: the orphan root itself. Now holders inside an accepted orphan root's tree fold into that root (only roots are returned; taskkill /T reaps descendants), and live-parent backends defer to the ancestry check instead of refusing outright. Anything outside an orphan tree still refuses. 3. Tests for the mixed shapes: root+managed-runtime child, grandchild depth, non-descendant stray alongside an orphan root (still refuses), descendant exited mid-classify. E2E on a real Windows box: spawned a detached backend-shaped orphan that itself spawned children (3 python descendants); the scanner-shaped mixed holder set classified to [root], taskkill /T reaped root and all descendants. The live Desktop backend on the box still classified None (refusal preserved). The first E2E attempt caught exactly the trampoline/worker case the mocks missed — the live worker re-execs with the same backend argv and a live parent — which is what part 2 fixes. Co-Authored-By: 4adwentures <296413879+4adwentures@users.noreply.github.com>
…te trees without pre-signalling Follow-up to NousResearch#82179 addressing helix4u's review comment (NousResearch#82179 issuecomment-5229441571). Three parts: 1. Desktop teardown (salvaged from NousResearch#77436, @4adwentures): the update hand-off's releaseBackendLock() sent SIGTERM to the primary backend BEFORE taskkill /T. If the launcher exits first, Windows can no longer enumerate its descendants and they survive holding the venv — the Electron path that creates the orphan NousResearch#82179 then has to repair. New stopBackendTreesForUpdate() tree-kills the live root first, with the behavioral vitest from NousResearch#77436. The scanner half of NousResearch#77436 is deliberately NOT taken (superseded by NousResearch#82158's full-cmdline scan). 2. Tree-aware orphan classification: _orphaned_desktop_backend_pids() previously refused the whole holder set when any holder had a live parent. But the scanner legitimately returns an orphaned serve root AND its descendants (the venv trampoline's uv-managed interpreter worker — which carries the same backend argv — plus .hermes-runtime children). Those have a live parent: the orphan root itself. Now holders inside an accepted orphan root's tree fold into that root (only roots are returned; taskkill /T reaps descendants), and live-parent backends defer to the ancestry check instead of refusing outright. Anything outside an orphan tree still refuses. 3. Tests for the mixed shapes: root+managed-runtime child, grandchild depth, non-descendant stray alongside an orphan root (still refuses), descendant exited mid-classify. E2E on a real Windows box: spawned a detached backend-shaped orphan that itself spawned children (3 python descendants); the scanner-shaped mixed holder set classified to [root], taskkill /T reaped root and all descendants. The live Desktop backend on the box still classified None (refusal preserved). The first E2E attempt caught exactly the trampoline/worker case the mocks missed — the live worker re-execs with the same backend argv and a live parent — which is what part 2 fixes. Co-Authored-By: 4adwentures <296413879+4adwentures@users.noreply.github.com>
…te trees without pre-signalling Follow-up to NousResearch#82179 addressing helix4u's review comment (NousResearch#82179 issuecomment-5229441571). Three parts: 1. Desktop teardown (salvaged from NousResearch#77436, @4adwentures): the update hand-off's releaseBackendLock() sent SIGTERM to the primary backend BEFORE taskkill /T. If the launcher exits first, Windows can no longer enumerate its descendants and they survive holding the venv — the Electron path that creates the orphan NousResearch#82179 then has to repair. New stopBackendTreesForUpdate() tree-kills the live root first, with the behavioral vitest from NousResearch#77436. The scanner half of NousResearch#77436 is deliberately NOT taken (superseded by NousResearch#82158's full-cmdline scan). 2. Tree-aware orphan classification: _orphaned_desktop_backend_pids() previously refused the whole holder set when any holder had a live parent. But the scanner legitimately returns an orphaned serve root AND its descendants (the venv trampoline's uv-managed interpreter worker — which carries the same backend argv — plus .hermes-runtime children). Those have a live parent: the orphan root itself. Now holders inside an accepted orphan root's tree fold into that root (only roots are returned; taskkill /T reaps descendants), and live-parent backends defer to the ancestry check instead of refusing outright. Anything outside an orphan tree still refuses. 3. Tests for the mixed shapes: root+managed-runtime child, grandchild depth, non-descendant stray alongside an orphan root (still refuses), descendant exited mid-classify. E2E on a real Windows box: spawned a detached backend-shaped orphan that itself spawned children (3 python descendants); the scanner-shaped mixed holder set classified to [root], taskkill /T reaped root and all descendants. The live Desktop backend on the box still classified None (refusal preserved). The first E2E attempt caught exactly the trampoline/worker case the mocks missed — the live worker re-execs with the same backend argv and a live parent — which is what part 2 fixes. Co-Authored-By: 4adwentures <296413879+4adwentures@users.noreply.github.com>
…te trees without pre-signalling Follow-up to NousResearch#82179 addressing helix4u's review comment (NousResearch#82179 issuecomment-5229441571). Three parts: 1. Desktop teardown (salvaged from NousResearch#77436, @4adwentures): the update hand-off's releaseBackendLock() sent SIGTERM to the primary backend BEFORE taskkill /T. If the launcher exits first, Windows can no longer enumerate its descendants and they survive holding the venv — the Electron path that creates the orphan NousResearch#82179 then has to repair. New stopBackendTreesForUpdate() tree-kills the live root first, with the behavioral vitest from NousResearch#77436. The scanner half of NousResearch#77436 is deliberately NOT taken (superseded by NousResearch#82158's full-cmdline scan). 2. Tree-aware orphan classification: _orphaned_desktop_backend_pids() previously refused the whole holder set when any holder had a live parent. But the scanner legitimately returns an orphaned serve root AND its descendants (the venv trampoline's uv-managed interpreter worker — which carries the same backend argv — plus .hermes-runtime children). Those have a live parent: the orphan root itself. Now holders inside an accepted orphan root's tree fold into that root (only roots are returned; taskkill /T reaps descendants), and live-parent backends defer to the ancestry check instead of refusing outright. Anything outside an orphan tree still refuses. 3. Tests for the mixed shapes: root+managed-runtime child, grandchild depth, non-descendant stray alongside an orphan root (still refuses), descendant exited mid-classify. E2E on a real Windows box: spawned a detached backend-shaped orphan that itself spawned children (3 python descendants); the scanner-shaped mixed holder set classified to [root], taskkill /T reaped root and all descendants. The live Desktop backend on the box still classified None (refusal preserved). The first E2E attempt caught exactly the trampoline/worker case the mocks missed — the live worker re-execs with the same backend argv and a live parent — which is what part 2 fixes. Co-Authored-By: 4adwentures <296413879+4adwentures@users.noreply.github.com>
…te trees without pre-signalling Follow-up to NousResearch#82179 addressing helix4u's review comment (NousResearch#82179 issuecomment-5229441571). Three parts: 1. Desktop teardown (salvaged from NousResearch#77436, @4adwentures): the update hand-off's releaseBackendLock() sent SIGTERM to the primary backend BEFORE taskkill /T. If the launcher exits first, Windows can no longer enumerate its descendants and they survive holding the venv — the Electron path that creates the orphan NousResearch#82179 then has to repair. New stopBackendTreesForUpdate() tree-kills the live root first, with the behavioral vitest from NousResearch#77436. The scanner half of NousResearch#77436 is deliberately NOT taken (superseded by NousResearch#82158's full-cmdline scan). 2. Tree-aware orphan classification: _orphaned_desktop_backend_pids() previously refused the whole holder set when any holder had a live parent. But the scanner legitimately returns an orphaned serve root AND its descendants (the venv trampoline's uv-managed interpreter worker — which carries the same backend argv — plus .hermes-runtime children). Those have a live parent: the orphan root itself. Now holders inside an accepted orphan root's tree fold into that root (only roots are returned; taskkill /T reaps descendants), and live-parent backends defer to the ancestry check instead of refusing outright. Anything outside an orphan tree still refuses. 3. Tests for the mixed shapes: root+managed-runtime child, grandchild depth, non-descendant stray alongside an orphan root (still refuses), descendant exited mid-classify. E2E on a real Windows box: spawned a detached backend-shaped orphan that itself spawned children (3 python descendants); the scanner-shaped mixed holder set classified to [root], taskkill /T reaped root and all descendants. The live Desktop backend on the box still classified None (refusal preserved). The first E2E attempt caught exactly the trampoline/worker case the mocks missed — the live worker re-execs with the same backend argv and a live parent — which is what part 2 fixes. Co-Authored-By: 4adwentures <296413879+4adwentures@users.noreply.github.com>
…te trees without pre-signalling Follow-up to NousResearch#82179 addressing helix4u's review comment (NousResearch#82179 issuecomment-5229441571). Three parts: 1. Desktop teardown (salvaged from NousResearch#77436, @4adwentures): the update hand-off's releaseBackendLock() sent SIGTERM to the primary backend BEFORE taskkill /T. If the launcher exits first, Windows can no longer enumerate its descendants and they survive holding the venv — the Electron path that creates the orphan NousResearch#82179 then has to repair. New stopBackendTreesForUpdate() tree-kills the live root first, with the behavioral vitest from NousResearch#77436. The scanner half of NousResearch#77436 is deliberately NOT taken (superseded by NousResearch#82158's full-cmdline scan). 2. Tree-aware orphan classification: _orphaned_desktop_backend_pids() previously refused the whole holder set when any holder had a live parent. But the scanner legitimately returns an orphaned serve root AND its descendants (the venv trampoline's uv-managed interpreter worker — which carries the same backend argv — plus .hermes-runtime children). Those have a live parent: the orphan root itself. Now holders inside an accepted orphan root's tree fold into that root (only roots are returned; taskkill /T reaps descendants), and live-parent backends defer to the ancestry check instead of refusing outright. Anything outside an orphan tree still refuses. 3. Tests for the mixed shapes: root+managed-runtime child, grandchild depth, non-descendant stray alongside an orphan root (still refuses), descendant exited mid-classify. E2E on a real Windows box: spawned a detached backend-shaped orphan that itself spawned children (3 python descendants); the scanner-shaped mixed holder set classified to [root], taskkill /T reaped root and all descendants. The live Desktop backend on the box still classified None (refusal preserved). The first E2E attempt caught exactly the trampoline/worker case the mocks missed — the live worker re-execs with the same backend argv and a live parent — which is what part 2 fixes. Co-Authored-By: 4adwentures <296413879+4adwentures@users.noreply.github.com>
What does this PR do?
Fixes the Windows GUI-updater dead-end where an update aborts with "Hermes is still running. Close all Hermes windows and try the update again." even though the user has zero windows open. Salvages #81327 intact underneath (authorship preserved) and adds the missing half on top.
Symptom (real logs, 2026-08-09, two consecutive failed updates)
…followed by the fallback bootstrap dying on the same lock:
Root cause
The Desktop→hermes-setup handoff can lose the teardown race: Electron SIGTERMs its backend and quits, but the
python.exe -m hermes_cli.main servebackend (and its managed.hermes-runtimeinterpreter child) survives. At that point the Desktop is gone — nothing will ever respawn that backend — buthermes update's venv-holder guard treats everyservebackend as "the Desktop supervises this, killing it is futile" and refuses. That assumption is only true while the Desktop is alive.Fix
Two commits:
Salvaged from fix(install): stop managed runtime child trees on Windows #81327 (@helix4u, commit intact):
scripts/install.ps1's venv sweep stops each verified venv process with its full child tree (taskkill /T /F) instead ofStop-Processon the parent only, so a managed-runtime child can't keep holding the install (force_kill_other_hermes() only kills hermes.exe, misses pythonw.exe backend → venv rebuild fails with "Access denied" on Windows #70026). Includes the behavioral regression test.New —
hermes_cli/update_cmd.py: the venv-holder guard now classifies remaining holders with_orphaned_desktop_backend_pids():serve/dashboardbackend whose supervising parent is provably dead (parent PID gone, or recycled — parent created after the child) is an orphaned straggler: reap its tree via the new_stop_process_trees()(taskkill /T /F, mirroring the Desktop'sforceKillProcessTree), re-scan, and proceed.--forcestill doesn't bypass;--force-venvsemantics unchanged.Related Issue
Related to #70026 (managed-runtime child holding the install) and the #39828 family (Desktop handoff race).
Type of Change
How to Test
pytest tests/hermes_cli/test_update_orphan_backend_reap.py tests/hermes_cli/test_update_venv_health.py tests/hermes_cli/test_scan_venv_blockers.py -o "addopts=--timeout-method=thread"— 41 passed.scripts/run_tests.sh tests/test_install_ps1_venv_process_tree.py(from fix(install): stop managed runtime child trees on Windows #81327).E2E verification (real Windows 11 box)
... -m hermes_cli.main serve --host 127.0.0.1): classifier returned[pid],_stop_process_treeskilled it — verified dead via psutil.None(refusal preserved).None(refusal preserved).Checklist
_is_windows()gating; no POSIX behavior change