fix(serve): Windows parent-death watchdog false-triggers due to ppid mismatch - #83604
Closed
LKrystalL wants to merge 1 commit into
Closed
fix(serve): Windows parent-death watchdog false-triggers due to ppid mismatch#83604LKrystalL wants to merge 1 commit into
LKrystalL wants to merge 1 commit into
Conversation
…k instead of ppid comparison On Windows, os.getppid() can differ from HERMES_PARENT_PID even when the parent (Electron desktop) is alive — intermediate launcher/shell processes change the apparent parent PID. The original ppid-based check would false- trigger immediately, causing the backend to exit before desktop could connect (exit code 0, no crash). Replaced with OpenProcess + GetExitCodeProcess for direct parent-process aliveness check on win32, with a 2x poll grace period after spawn to let the process tree stabilise. POSIX path unchanged.
Contributor
|
Thanks @LKrystalL — correct diagnosis of the same regression (the #83406 watchdog's PPID comparison never matches through the Windows venv shim). This landed on main via #83615, salvaged from #83556 which was submitted ~1h earlier and probes via the existing psutil-backed |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
After updating to v0.20.0 (commit 33f8e96), Hermes Desktop on Windows fails to start with:
The backend process (
hermes serve) starts, printsHERMES_BACKEND_READY port=XXXXX, then exits with code 0 within ~2 seconds — before the desktop Electron shell can complete its health-check handshake.Root Cause
The newly introduced
_start_parent_death_watchdog()inhermes_cli/web_server.pycomparesos.getppid()againstHERMES_PARENT_PID(the Electron process PID) to detect whether the spawning parent has died. On Windows, the child process'sgetppid()often differs from the Electron PID because of intermediate launcher/shell processes in the spawn chain. The watchdog therefore immediately misidentifies the parent as dead and callsos._exit(0).The
_is_serve_orphanedcheck:On POSIX this works reliably because
fork()+exec()preserves the ppid. On Windows,spawn()often inserts intermediate processes.Fix
On Windows, replace the ppid-comparison check with a direct process-aliveness check via
OpenProcess+GetExitCodeProcess. This is the correct Windows idiom for determining whether a process is still running, regardless of the process-tree topology.A 2× poll grace period is added after spawn to let the process tree stabilise before the first aliveness probe.
POSIX behaviour is unchanged.
Verification
_demo()self-check passes_is_windows_process_alive(os.getpid())→True_is_windows_process_alive(99999999)→FalseHERMES_DESKTOP=1 HERMES_PARENT_PID=$$ hermes.exe servenow stays alive (previously exited in <2s)test_start_server_keeps_bare_asyncio_run_on_posixon Windows)Diff