fix(desktop): exclude respawned backends from venv-blocker scan (#77277) - #77772
fix(desktop): exclude respawned backends from venv-blocker scan (#77277)#77772RelaxJonh wants to merge 1 commit into
Conversation
…Research#77277) The Desktop in-app updater on Windows enters an infinite loop when updating: releaseBackendLockForUpdate kills the backend, but the Desktop app respawns it within seconds, and scanVenvBlockers reports the respawned process as a blocker. The user sees "Update aborted: another Hermes process is using this installation" with a different PID each time they retry. Root cause: scanVenvBlockers calls _detect_venv_python_processes() without excluding the Desktop app's own managed processes. After releaseBackendLockForUpdate kills the backend, the Desktop app's ensureBackend/startHermes respawns it, and the scanner picks it up as an external blocker. Fix: Add --exclude-children-of <PID> to _scan_venv_blockers.py. When the Desktop app invokes the scanner, it passes its own PID (process.pid), causing the scanner to exclude all descendants of the Electron main process. This covers both the original backend PIDs and any respawned ones. Changes: - hermes_cli/_scan_venv_blockers.py: Add argparse --exclude-children-of and _collect_descendant_pids() helper using psutil process tree walk. - apps/desktop/electron/venv-blocker-scan.ts: Add ScanOptions interface with excludeChildrenOf param, pass --exclude-children-of to subprocess. - apps/desktop/electron/main.ts: Pass process.pid as excludeChildrenOf to scanVenvBlockers in the applyUpdates preflight. - tests/hermes_cli/test_scan_venv_blockers.py: Update test helper to accept kwargs from _detect_venv_python_processes.
f0c7bd8 to
811a8a7
Compare
|
Closing after a premise-check against current main — the diagnosis was real and useful, but both halves have been overtaken:
The durable replacement for "whose process is this?" is identity, not ancestry: the spawn ledger (self-registered pid+create_time+purpose+spawner, landed via #91869/#92698) and, next, the gateway control socket's pause-for-update verb (#92091), which turns this whole scan-and-guess preflight into a negotiated drain. Thanks for the clear repro table in the PR — it's what made the premise-check quick. |
|
Closing after a premise-check against current main (part of the #91277 venv-holder slice 2 pass) — this was a reasonable fix against v0.19-era code, but main has since moved in a direction that conflicts with the mechanism here:
#77277 stays open — the durable fix for the respawn dance is the gateway control socket's |
Fixes #77277
Root Cause
The Desktop in-app updater on Windows enters an infinite loop:
releaseBackendLockForUpdatekills the backend, but the Desktop app respawns it within seconds, andscanVenvBlockersreports the respawned process as a blocker. The user sees "Update aborted: another Hermes process is using this installation" with a different PID each retry.The
waitForUpdateClearancegate (#73822) preventsstartHermes/spawnPoolBackendfrom respawning during the update, but on older versions (v0.19.0) the gate may not be present or may have race conditions. Even with the gate, the scanner itself has no way to distinguish the Desktop's own managed processes from external holders.Fix
Add
--exclude-children-of <PID>to_scan_venv_blockers.py. When the Desktop app invokes the scanner, it passes its own PID (process.pid), causing the scanner to exclude all descendants of the Electron main process via apsutilprocess tree walk. This covers both the original backend PIDs and any respawned ones.Changes
hermes_cli/_scan_venv_blockers.pyargparse--exclude-children-ofarg +_collect_descendant_pids()helperapps/desktop/electron/venv-blocker-scan.tsScanOptionsinterface, pass--exclude-children-ofto Python subprocessapps/desktop/electron/main.tsprocess.pidasexcludeChildrenOfinapplyUpdatespreflighttests/hermes_cli/test_scan_venv_blockers.pyTesting
test_scan_venv_blockers.pytests pass_collect_descendant_pidsuses psutil process tree walk — safe on non-Windows (returns empty set)parse_known_argsensures backward compatibility with CLI usage (no args = old behavior)