fix(gateway): spare supervised and recorded gateways from the orphan reaper on all platforms - #86658
Conversation
_reap_unsupervised_gateway_orphans() kills every gateway PID found by find_gateway_pids() on hosts without systemd (macOS launchd, Windows Scheduled Task). This includes service-managed gateways that are NOT orphans — they are supervised by launchd/systemd and should never be killed during a stale-process sweep. Add own |= _get_service_pids() to the exclusion set before scanning, so launchd/systemd-supervised gateways are preserved. True orphans (reparented leftovers not present in launchctl/systemctl) are still found and reaped, preserving the #77276 protection. Fixes #85344 (macOS launchd gateway killed by desktop serve startup) Fixes #85044 (Windows Scheduled Task gateway killed by desktop serve) Fixes #84855 (Permission denied to kill orphaned gateway PID) Fixes #85368 (gateway process repeatedly killed, messaging offline)
…per on Windows The orphan reaper kills a healthy gateway (and its Scheduled-Task bootstrap parent chain) every time the Desktop backend starts on Windows, because _get_service_pids() only implements systemd/launchd and returns an empty set on Windows — a supervised gateway is therefore indistinguishable from an unsupervised orphan. Exempt the recorded healthy gateway PID and its parent chain from the orphan scan on Windows, mirroring the macOS launchd exemption (#85913). The Scheduled-Task bootstrap's argv matches the gateway scan, so without exempting the parent chain killing the bootstrap takes the detached gateway down with it. Fixes #86098
…r to all platforms Compose the service-PID exclusion (#85743, RelaxJonh) and the recorded-PID + parent-chain exemption (#86100, arccat-114) into one cross-platform rule: - _get_service_pids() exclusion now runs unconditionally, not only under is_macos() — it is the authoritative "supervised" signal for launchd and any systemd unit visible on a host that got past the systemd gate. - The recorded-healthy-gateway (get_running_pid()) + parent-chain exemption now runs on every platform, not only Windows. A recorded, liveness-verified gateway is by definition not an orphan "the pidfile/runtime record can't see", so the reaper must never target it — this covers Windows Scheduled Task / Startup VBS supervision, standalone launcher-started gateways (the case #85743 alone would miss), and macOS/WSL equivalents. True orphans (no service registration, no valid runtime record) are still found and reaped, preserving the #51325/#75936 duplicate-port protection. Existing macOS regression tests updated to pin get_running_pid to None for their scenario; Windows regression tests from #86100 carry over unchanged. Bug class: #83683 (root), #86287, #86098, #85738, #85368, #85344, #85044, #84855, #84824, #84200.
|
Happy to test this once it merges — same setup as our #83683 verification: macOS 26.5 headless mini, gateway under launchd ( Test plan against this PR:
Will report both results back here or on #83683. |
alt-glitch
added
type/bug
|
@teknium1 — fair points, and the consolidation is the right call. I've split the relaunch half out so #83683 is fully covered by two independent, composable fixes. On the two critiques of #83720 — you're right on both:
On the parent-chain walk: Split done: I extracted the relaunch half into #86693 —
#83720 can be closed in favor of #86658 (reap) + #86693 (relaunch). Thanks for crediting the original diagnosis and for cherry-picking @RelaxJonh's and @arccat-114's work with authorship preserved — that's the right way to consolidate. Happy to help land #86693 or adjust either PR if reviewers want changes. |
૮ >ﻌ< ა ci reviewran on cda8373 — chore: add contributor email mapping for arccat-114
|
|
Reviewed this closely. The direction is right — excluding service-managed/recorded PIDs instead of a full-bail guard is the correct call, and it preserves the #51325/#75936 duplicate-port protection that my earlier 1. Windows protection hinges on 2. Unbounded parent-chain walk. The code adds every ancestor of the recorded gateway (up to PID 1) to the exclude set. Only the immediate supervisor bootstrap matters (its argv matches the gateway scan); the rest is broader than needed and makes the exclusion depend on 3. Test gap. The Windows tests always mock Better solution: I've pushed #86702, which keeps the service-PID + recorded-PID exclusions (preserving #51325) but (a) bounds the parent exclusion to the immediate supervisor, and (b) adds a supervisor-owned backstop in the orphan filter — any candidate whose parent chain reaches Like this PR, #86702 only fixes the reap half; the "never relaunches" half of #83683 is tracked separately in #86693. So the complete, robust fix is #86702 (reap) + #86693 (relaunch). Happy to fold the relaunch in if reviewers prefer a single PR, but keeping them separate matches the earlier feedback about not bundling. (Note: psutil is already a hard dependency of the codebase, so the parent-chain imports in both PRs are safe — not a concern.) |
|
Tested the merged fix live on our macOS headless-mini setup — results posted on #83683: dashboard/desktop-backend start no longer triggers the reap (gateway PID stable, zero shutdown spam), restart path clean on the new code. The "never relaunches" half (#86693) is the remaining piece for full closure. |
…han reaper on Windows (#83683) On Windows _get_service_pids() is empty (no systemd/launchd query), so a Scheduled-Task-supervised gateway whose gateway.pid record is missing or stale is invisible to both the service-PID and recorded-PID exclusions the reaper already applies (#86658) — and gets SIGTERM'd on every desktop open (#86098 class, pidfile-less path). Add a Windows-only backstop: any reaper candidate whose parent chain reaches services.exe (the Task Scheduler launches tasks under the services tree) is spared even with no pidfile. The backstop is deliberately inert on POSIX: every process there has PID 1 (launchd/init/systemd) in its ancestry — and a genuine orphan is reparented directly to PID 1 — so supervisor-name ancestry carries zero supervision signal and would disable the reaper entirely on macOS/WSL (#51325, #75936). POSIX supervised gateways are already covered pidfile-independently by the _get_service_pids() exclusion. Known limitation (fail-open, documented): if the Task-launched bootstrap parent has already exited, Windows does not reparent the gateway, the chain breaks before services.exe, and the gateway is treated as an orphan. Salvaged from #86702 by @EvanProgramming (authorship preserved); reduced to the genuinely-new Windows backstop — the PR's other two hunks were already merged on main via #86658 (one in a strictly stronger full-parent-chain form) and its POSIX ancestry checks were dropped as unsound (verified empirically: a true double-fork orphan's psutil parent IS launchd).
…han reaper on Windows (NousResearch#83683) On Windows _get_service_pids() is empty (no systemd/launchd query), so a Scheduled-Task-supervised gateway whose gateway.pid record is missing or stale is invisible to both the service-PID and recorded-PID exclusions the reaper already applies (NousResearch#86658) — and gets SIGTERM'd on every desktop open (NousResearch#86098 class, pidfile-less path). Add a Windows-only backstop: any reaper candidate whose parent chain reaches services.exe (the Task Scheduler launches tasks under the services tree) is spared even with no pidfile. The backstop is deliberately inert on POSIX: every process there has PID 1 (launchd/init/systemd) in its ancestry — and a genuine orphan is reparented directly to PID 1 — so supervisor-name ancestry carries zero supervision signal and would disable the reaper entirely on macOS/WSL (NousResearch#51325, NousResearch#75936). POSIX supervised gateways are already covered pidfile-independently by the _get_service_pids() exclusion. Known limitation (fail-open, documented): if the Task-launched bootstrap parent has already exited, Windows does not reparent the gateway, the chain breaks before services.exe, and the gateway is treated as an orphan. Salvaged from NousResearch#86702 by @EvanProgramming (authorship preserved); reduced to the genuinely-new Windows backstop — the PR's other two hunks were already merged on main via NousResearch#86658 (one in a strictly stronger full-parent-chain form) and its POSIX ancestry checks were dropped as unsound (verified empirically: a true double-fork orphan's psutil parent IS launchd).
…han reaper on Windows (NousResearch#83683) On Windows _get_service_pids() is empty (no systemd/launchd query), so a Scheduled-Task-supervised gateway whose gateway.pid record is missing or stale is invisible to both the service-PID and recorded-PID exclusions the reaper already applies (NousResearch#86658) — and gets SIGTERM'd on every desktop open (NousResearch#86098 class, pidfile-less path). Add a Windows-only backstop: any reaper candidate whose parent chain reaches services.exe (the Task Scheduler launches tasks under the services tree) is spared even with no pidfile. The backstop is deliberately inert on POSIX: every process there has PID 1 (launchd/init/systemd) in its ancestry — and a genuine orphan is reparented directly to PID 1 — so supervisor-name ancestry carries zero supervision signal and would disable the reaper entirely on macOS/WSL (NousResearch#51325, NousResearch#75936). POSIX supervised gateways are already covered pidfile-independently by the _get_service_pids() exclusion. Known limitation (fail-open, documented): if the Task-launched bootstrap parent has already exited, Windows does not reparent the gateway, the chain breaks before services.exe, and the gateway is treated as an orphan. Salvaged from NousResearch#86702 by @EvanProgramming (authorship preserved); reduced to the genuinely-new Windows backstop — the PR's other two hunks were already merged on main via NousResearch#86658 (one in a strictly stronger full-parent-chain form) and its POSIX ancestry checks were dropped as unsound (verified empirically: a true double-fork orphan's psutil parent IS launchd).
5 participants
Summary
The gateway orphan reaper (
_reap_unsupervised_gateway_orphans()) no longer kills supervised or recorded-healthy gateways on Desktop backend (re)start — on any platform. Root cause: the reaper's "supervised" check only knew systemd, so macOS launchd gateways and Windows Scheduled-Task / Startup-VBS gateways (plus standalone launcher-started gateways with a valid runtime record) were indistinguishable from orphans and were SIGTERM'd/SIGKILL'd every time aHERMES_DESKTOP=1serve backend started — leaving messaging (WeChat/QQ/Telegram/Signal/Discord) silently offline, or under launchdKeepAliveproducing infinite kill loops and in-flight request loss.Consolidated salvage of the two complementary open fixes, generalized to fix the whole class:
_get_service_pids()(launchd/systemd-managed PIDs) from the reap set. Cherry-picked with authorship preserved.get_running_pid()) and its parent chain so the Scheduled-Task bootstrap (whosegateway runargv matches the scan) isn't killed out from under the detached gateway. Cherry-picked with authorship preserved, including 2 Windows regression tests.is_macos()/is_windows()gates: a recorded, liveness-verified gateway is by definition not an orphan "the pidfile/runtime record can't see", on any platform. This also covers the standalone-gateway case (Desktop restart reaps the live gateway but never relaunches it (WeChat/QQ go silent) — regression #83683 comments) that a service-PID-only exclusion would miss.Why not #83720: @EvanProgramming's PR pioneered the supervisor-guard direction and drove the diagnosis on #83683 — credited. But its full-bail
_gateway_has_active_supervisor()short-circuit disables orphan reaping entirely whenever any supervisor is present (regressing the #51325/#75936 duplicate-port protection), and it bundles an independent desktop-boot relaunch feature (new config key + env var + web_server lifespan hook) that belongs in its own reviewed change. The exclusion-based fix here keeps true-orphan reaping working while sparing everything supervised or recorded.Changes
hermes_cli/gateway.py:_reap_unsupervised_gateway_orphans()— service-managed PIDs and the recorded gateway PID + parent chain are excluded from the orphan set on all platforms.tests/hermes_cli/test_gateway.py: +2 Windows regression tests (from fix(gateway): spare Scheduled-Task-supervised gateway from orphan reaper on Windows #86100); existing macOS tests updated to pinget_running_pidfor their scenario.contributors/emails/: mapping for arccat-114.Validation
Targeted tests:
test_gateway.py,test_spawn_gateway_restart_reap.py,test_gateway_windows.py,test_gateway_proc_fallback.py— 28 passed, 2 skipped. E2E with real imports in an isolatedHERMES_HOME: recorded-PID spared / service-PID spared / true orphan reaped — all pass.Fixes #83683. Also resolves #86287, #86098, #85738, #85368, #85344, #85044, #84855, #84824, #84200.
Supersedes #85743, #86100, #83720 (reap half; the desktop-boot relaunch half of #83720 can be proposed separately).
Infographic