fix(serve): self-reap the desktop backend when its parent dies - #79112
Closed
Rmohid wants to merge 1 commit into
Closed
fix(serve): self-reap the desktop backend when its parent dies#79112Rmohid wants to merge 1 commit into
Rmohid wants to merge 1 commit into
Conversation
Three `hermes_cli.main serve --host 127.0.0.1 --port 0` processes were found
orphaned at ppid 1 from a single ~3-minute restart burst, each holding a live
127.0.0.1 LISTEN socket with zero connections and 53-138 MB resident.
The spawner is the desktop Electron app (apps/desktop/electron/
backend-command.ts serveBackendArgs emits exactly that argv). Its teardown is
correct -- before-quit does SIGTERM then forceKillProcessTree over the primary
backend and the pool -- but structurally cannot cover this: on force-quit /
SIGKILL / fatal GPU abort that handler never runs, and main.ts says so itself
("FATAL GPU aborts skip before-quit"). No macOS crash report exists for the
window, consistent with SIGKILL. macOS has no PR_SET_PDEATHSIG (documented in
tools/mcp_stdio_watchdog.py), so the kernel reparents the backend to pid 1
instead of reaping it.
No parent-side fix can close this, because the parent's code is exactly what
did not run. So the child reaps itself, using the same watchdog idiom as
tui_gateway/slash_worker.py: record ppid at startup, exit once it changes.
Gated to the one launch shape that can leak -- headless `serve` only (never the
interactive `dashboard`, which shares cmd_dashboard), HERMES_DESKTOP=1 only (a
deliberate `nohup hermes serve &` legitimately reparents to pid 1), and POSIX
only (the profile re-exec is os.execvpe here, preserving pid/ppid; on Windows
it is subprocess.Popen and the recorded ppid would be stale). Started after the
re-exec for that same reason.
Verified end to end against a real serve backend on Darwin 25.5.0, both arms:
with HERMES_DESKTOP=1 the backend self-reaped 1s after its parent was SIGKILLed
and released its LISTEN socket; without it the backend survived at ppid 1,
reproducing the leak and confirming standalone daemons stay protected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
Resolved on main by PR #83406 (rebase-merged), which carries the parent-death watchdog + group-kill (from #73066) and the Desktop-boot reap of already-orphaned serve backends. Special credit here: this PR was the EARLIEST submission of the serve orphan watchdog idea (July 9), predating the implementation that ultimately landed — the design direction was yours first. Thank you! |
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.
What does this PR do?
Makes the headless
hermes servebackend reap itself when its parent dies, closing the one orphan path that no parent-side fix can reach.The desktop app spawns its backend as
hermes serve --host 127.0.0.1 --port 0and tears it down inbefore-quit. That teardown is correct, and #76245 / #76244 make it more reliable still. But all three live on the parent path, and there is a case where the parent's code never runs at all: force-quit,SIGKILL, or a fatal GPU abort.main.tsalready acknowledges this in a comment — "FATAL GPU aborts skip before-quit".When that happens on macOS there is no
PR_SET_PDEATHSIG(this repo documents the constraint verbatim intools/mcp_stdio_watchdog.py), so the kernel reparents the backend to pid 1 instead of reaping it. It keeps its127.0.0.1LISTEN socket and its resident memory forever. That is thePPID=1accumulation in #61349.Since the parent's code is exactly what did not run, the fix has to be child-side.
This is not a new mechanism for this repo.
tui_gateway/slash_worker.pyalready solves precisely this with a parent-death watchdog, andtests/test_slash_worker_watchdog.pypins it. #61349 reports orphans from bothserveandslash_worker—slash_workeris already covered by this idiom;servenever was. This PR extends the proven pattern toserve, deliberately mirroring the existing code and tests rather than inventing anything.Relationship to the other open orphan issues
This is a complement, not a replacement:
Related Issue
Addresses the force-quit / abort path of #61349, and reduces the pile-up behind #78872 and #78821.
To be precise about scope: #61349 likely has more than one cause, and the graceful-quit paths belong to #76245 / #76244. This PR does not supersede those — it covers the case where no teardown code runs at all. I've deliberately written "addresses" rather than "fixes" for that reason.
Type of Change
Changes Made
hermes_cli/main.py(+72, purely additive — no existing lines changed):_serve_is_orphaned(original_ppid, getppid=os.getppid)— same predicate/signature asslash_worker._is_orphaned._should_start_serve_watchdog(headless_backend, env, os_name)— the gate, as a pure function so it is directly testable._start_serve_parent_death_watchdog(original_ppid)— daemon thread;os._exit(0)once orphaned._env_float(...)+_SERVE_WATCHDOG_POLL_S(HERMES_SERVE_WATCHDOG_POLL_S, default 2.0s), mirroring theslash_workerhelper so a typo'd env var can't raise at import.cmd_dashboard.tests/hermes_cli/test_serve_parent_death_watchdog.py(new, 9 tests).The gate
cmd_dashboardbacks bothdashboardandserve, so the watchdog is gated on all three of:_headless_backend—serveonly. A human's foregroundhermes dashboardmust never self-reap.HERMES_DESKTOP=1— the desktop's own backend only. A deliberatenohup hermes serve &legitimately reparents to pid 1 when its shell exits; killing that would be a regression, not a fix. This env var is already load-bearing here —cmd_dashboardbranches on it a few lines below.tools/mcp_tool.pyuses.Why the call sits after the re-exec
The named-profile re-exec is
os.execvpeon POSIX, which preserves pid/ppid, so a ppid recorded after it stays valid. On Windows that same branch issubprocess.Popen, where it would not — hence both the placement and the POSIX gate. A test asserts the ordering so a future refactor can't silently move it above the re-exec.How to Test
Both arms, against a real backend (not a mock). Substitute any Python with the repo importable:
1. Treatment — desktop-shaped launch self-reaps:
2. Control — a standalone daemon is left alone. Same script with
HERMES_DESKTOPunset: the child survives atPPID=1. This both reproduces the original bug and demonstrates the gate protects a deliberatenohup hermes serve &.3. Unit + integration tests:
bash scripts/run_tests.sh --files "tests/hermes_cli/test_serve_parent_death_watchdog.py:tests/hermes_cli/test_serve_command.py:tests/test_slash_worker_watchdog.py"Results on my machine (macOS 15 / Darwin 25.5.0, Python 3.11.14)
Checklist
Code
pytest tests/ -qand all tests pass — partially; see note belowDocumentation & Housekeeping
cli-config.yaml.example— N/A (no config keys; the one env knob is test-only tuning with a safe default)CONTRIBUTING.md/AGENTS.md— N/A🤖 Generated with Claude Code