fix(tui-gateway): reap leaked slash_worker processes/fds on disconnect, create-race, and parent-death by banditburai · Pull Request #35626 · NousResearch/hermes-agent

banditburai · 2026-05-31T01:48:31Z

Dashboard chat sessions leaked tui_gateway.slash_worker subprocesses — plus their pipe fds and PTY master fds — whenever a session disconnected, was abandoned, or its parent gateway died. This PR adds a teardown mechanism for each of the three independent producers (one narrow C3 residual disclosed under Known limitations); it is the leak cause (close + reap). The launchd fd-ceiling raise + self-heal ships separately in the sibling PR fix/gateway-fd-limit-hardening (Refs #24775).

Problem

Leaked workers accumulate without bound and drag the host down. Observed: 128 stale workers swap-pinning a 7.8 GB box (#21467); fd exhaustion on macOS launchd (#24775); a per-turn handler leak (#21370); a dashboard hang from PTY-disconnect leaks behind a reverse proxy returning 524 (#32377); general lifecycle-gap fragility under intensive use (#22855). There are three distinct producers, and fixing any one alone leaves the leak in place.

Root cause → mechanism → file → commit

Cause	What leaked	Previous behavior	Fix	File	Commit
C1 — disconnect never reaped (keystone)	session + its worker	`ws.py` `finally` only re-pointed the transport back to stdio; the session (and worker) lived on	`_close_sessions_for_transport` reaps `close_on_disconnect` sessions / re-points the rest, dispatched via `asyncio.to_thread` (the teardown does a blocking `worker.close()` + sync DB write — inline would stall the uvicorn loop for every other live connection); funnels through one idempotent `_close_session_by_id`	`ws.py`, `server.py`	`0431e38eb` `6ce86df30` `8912e62b3` `b01b054d0`
C2 — create/close orphan race	a worker built after its session was reaped	spawn sites stored the worker unconditionally; a concurrent teardown left it orphaned	`_attach_worker` stores the worker iff `_sessions.get(sid) is session` (identity) under `_sessions_lock`, else closes it — applied at all 4 spawn sites incl. the post-turn `_restart_slash_worker`	`server.py`	`8912e62b3` `05007e68e`
C3 — parent death (crash / SIGKILL / auto-update; `atexit` never runs)	worker, orphaned PTY grandchildren, PTY master fd	nothing detected a dead parent; `close()` signalled only the PTY leader; master fd was dropped without `force`	(i) psutil parent-death watchdog (compares to original `getppid()` + `create_time` PID-reuse guard, drains in-flight then `os._exit(0)`); (ii) PTY group-kill via `os.killpg` on the child's `setsid` group (guarded `!= os.getpgrp()` so it never signals the gateway's own group) + `proc.close(force=True)` to close the master fd; (iii) idempotent `_SlashWorker.close()` (`_closed` flag + post-SIGKILL zombie reap + stdio close)	`slash_worker.py`, `pty_bridge.py`, `server.py`	`dc0add6ff` `e637c7b41` `9339e47a4`

Two supporting changes (detectors / defense-in-depth, not root causes):

uvicorn ws_ping_interval/ws_ping_timeout = 20s — server-initiated pings turn a half-open socket (reverse-proxy 524, dropped tunnel) into a WebSocketDisconnect within ~20–40 s (one ping interval + one timeout), so the C1 disconnect path can run instead of the socket lingering silently. web_server.py (8d36d42fd).
Idle-session reaper — a daemon thread that evicts only sessions failing every liveness check (not running, no pending kind, agent ready, transport dead, both last_active and created_at older than a 6 h TTL). A last-resort backstop for any disconnect that slips past the WS finally. server.py (d2b4f7b50; see Known limitations).

Cross-cutting note

All teardown paths — session.close RPC, atexit shutdown, WS disconnect, idle eviction — now funnel through the single idempotent _close_session_by_id. Idempotency (via _finalized on the session and _closed on the worker) is the explicit safety contract: concurrent/double teardown is a no-op, never a double-kill.

Resolves

Fixes #21370 — the worker is per-session (reused across turns, not respawned per turn); the leak was that the session + worker were never reaped on disconnect (C1), and the post-turn _restart_slash_worker could orphan its replacement worker (C2). Both are now closed.
Fixes #21467 — worker accumulation / swap-pinning: the same reap path bounds steady-state count; the idle reaper backstops it.
Fixes #24775 — fd exhaustion. PR1 fixes the leak cause (PTY master-fd close + zombie reap + session/worker teardown). The launchd fd-cap raise is mitigation and ships in the sibling PR (Refs #24775).
Fixes #32377 — PTY chat disconnect / 524 behind reverse proxy: ws_ping detects the half-open socket → C1 reap → PTY group-kill reaps the grandchildren.
Addresses #22855 — lifecycle gaps / fragility: the watchdog + C1 reap + C2 guard cover the common-case gaps. This PR does not add the tools.process_registry registration the issue proposed, so global teardown still can't enumerate live workers, and the startup-window race noted below remains; hence Addresses rather than Fixes.
Supersedes #21401 — builds on its disconnect-close approach (same files: ws.py, server.py, ChatSidebar.tsx), adding off-loop teardown (asyncio.to_thread) and the _attach_worker create-race guard.
Supersedes #32483 — carries forward its PTY process-group reaping and adds idempotent close(). For parent-death detection it uses a cross-platform psutil watchdog (create_time-based, PID-reuse-safe) in place of [codex] fix(tui): reap dashboard slash workers #32483's Linux-only PR_SET_PDEATHSIG; the trade is slightly higher detection latency on Linux in exchange for covering macOS, the platform in these reports.

Known limitations

Watchdog startup-window race: if the gateway dies during the worker's pre-getppid() import window, the worker anchors to init and never self-terminates. Narrow; unreached by the registry (see Bug: slash_worker lifecycle gaps and system fragility observed during intensive Dashboard usage (#21370) #22855). Principled fix (deferred): pass the gateway PID to the worker at spawn instead of relying on getppid(). The other two C3 mechanisms (PTY group-kill, idempotent close) are unaffected.
Idle reaper evicts long-idle local stdio sessions: _transport_is_dead treats the stdio transport as always-dead, so a local stdio session idle > 6 h (not running, no pending) will be reclaimed. Bounded (local-only, 6 h TTL, multiple exemptions); accepted for now.
The PDEATHSIG → psutil swap above is a behavioral characteristic (Linux detection latency), not a regression.

Tests

Each commit ships its own tests (unit + a live parent-death integration test under -m integration).

Reviewer guidance

Hot spots worth the closest look: the off-loop asyncio.to_thread teardown (no event-loop reentrancy; DB write off the loop thread); _attach_worker's is identity check (not ==) covering the restart site; the os.killpg != os.getpgrp() self-group guard; the watchdog's in-flight drain before os._exit(0). Sibling: fix/gateway-fd-limit-hardening raises the macOS launchd fd ceiling (Refs #24775).

alt-glitch · 2026-05-31T02:08:48Z

Competing with #32483, #22863, #25765 — all address the slash_worker subprocess leak family (#21370, #24775, #32377, #22855, #21467). This PR is the most comprehensive, addressing all three independent leak producers (disconnect, create-race, parent-death). Sibling of #35627 (fd ceiling raise).

tonydwb

Code Review Summary

Verdict: Approved ✅

Reviewed Changes

Comprehensive TUI gateway slash_worker leak fix. Three independent producers: (C1) disconnect reaping, (C2) create-close orphan race guard, (C3) parent-death watchdog. Plus uvicorn ws_ping, idle-session reaper, PTY group-kill.

✅ Looks Good

Correctness: Each producer is independently addressed. The _attach_worker identity check (is) correctly handles concurrent spawns. PTY group-kill is guarded with != os.getpgrp(). Watchdog uses psutil create_time for PID-reuse safety. Idempotent close via _closed/_finalized flags prevents double-kill.
Off-loop teardown: asyncio.to_thread prevents blocking the uvicorn loop during worker close + DB sync.
Well-documented: Root cause table, test coverage per commit, known limitations and startup-window race disclosure.
Edge cases: Zombie reap after SIGKILL, master fd close (force=True), GC of task handles, concurrent disconnect+spawn.
No security concerns: No credential exposure. PTY group-kill self-guard prevents signalling gateway's own group.

Reviewed by Hermes Agent

… idempotent) Task: swl-qrf.2

Adds a reentrant _sessions_lock and a single locked teardown helper. A new _attach_worker(sid, session, worker) stores the worker only while sid still maps to the session, else closes it — closing the create/close orphan race at the three spawn sites (_build, _init_session, on-demand slash.exec). _build's finally no longer double-closes the worker (attach owns that now). Task: swl-qrf.3

…session_by_id Task: swl-qrf.4

…nsport Task: swl-qrf.5

…ching transport) The disconnect finally only re-pointed each owned session's transport to stdio, so close_on_disconnect sidecar sessions (and their slash_worker subprocesses) survived every clean tab-close until atexit — the dominant leak. Now delegate to _close_sessions_for_transport off the event loop (to_thread) so the blocking worker.close()/DB write can't stall other live WS connections. Task: swl-qrf.6

Task: swl-qrf.7

…tchdog Daemon thread polls _is_orphaned (original ppid check + psutil create_time PID-reuse guard, no PR_SET_PDEATHSIG). On orphan, drains an in-flight command up to a grace window then os._exit(0). Started before the HermesCLI build to cover the spawn window. Task: swl-qrf.8

Spawns a gateway stand-in that forks a watchdog-armed worker; the worker announces its pid only after arming (so the kill can't precede arming and anchor the watchdog to init). SIGKILLs the parent and asserts the orphan self-exits within a generous multiple of the sub-second poll+grace. Marked integration + live_system_guard_bypass. Task: swl-qrf.9

…ousResearch#24135) close() escalated SIGHUP->SIGTERM->SIGKILL against the leader only, leaking any grandchildren it spawned (NousResearch#32377). Capture the child's pgid at construction (never the dashboard's own group) and signal the whole group via os.killpg, with a single-leader fallback when no pgid is available. Keeps the trailing proc.close(force=True) that closes the PTY master fd (the NousResearch#24775 fd-leak fix). Task: swl-qrf.10

Task: swl-qrf.11

…tions Daemon thread scans every 5min and evicts sessions only when ALL hold: not running, no pending input, agent finished starting, transport dead/detached, and both last_active and created_at older than the 6h TTL. Backstops any disconnect path that slips past the WS finally (NousResearch#21370/NousResearch#21467). Task: swl-qrf.12

The post-turn slash-worker restart stored the fresh worker with a bare session["slash_worker"] = ... assignment. That runs as a turn ends and `running` flips false — exactly when a close_on_disconnect reap can pop the session on another thread. The bare store then leaked the new worker (reaped only on gateway exit via the parent-death watchdog). Thread sid through _restart_slash_worker and route the store through the same _attach_worker store-iff-still-mapped guard as the three spawn sites, closing the worker instead of orphaning it when the session is gone. Refs NousResearch#21370

…module) The check-windows-footguns CI flags the two os.killpg calls in the PTY group-kill path. pty_bridge is already POSIX-only (it imports fcntl/ termios/ptyprocess at module top, and the SIGHUP/SIGTERM/SIGKILL loop already carries the same suppression), so these are intentionally platform-gated. Suppress per the checker's documented convention rather than swapping to psutil, which can't target the PTY process group the NousResearch#32377 grandchild-reaping fix relies on.

teknium1 · 2026-06-08T14:03:59Z

Salvaged into #42132. Your cross-platform parent-death watchdog commit (feat(slash-worker): self-terminate on parent death via create_time watchdog) was cherry-picked onto current main with authorship preserved (8b6a8f6) — it's guard 2 of the merged fix, and the right call over Linux-only PR_SET_PDEATHSIG. The rest of this PR's server.py/ws.py churn overlapped with the orphan-reaper that landed separately (#38591), so we took the two isolated, high-value commits rather than the full set. Thanks for the thorough work.

…ctive_list liveness (re-scoped onto current main) Salvaged from NousResearch#35626 (banditburai) and re-scoped after maintainers landed the parent-death watchdog (slash_worker.py) and PTY process-group teardown (pty_bridge.py) directly on main. Those pieces are intentionally NOT included here — this carries only what is still missing: - C1 disconnect reap: ws.py's `finally` only re-pointed the dead transport at stdio. `_close_sessions_for_transport` now reaps `close_on_disconnect` sessions and schedules the grace-reap for the rest, offloaded via `asyncio.to_thread` so the blocking worker.close() + DB write never stalls the uvicorn loop. - C2 create/close orphan race: `_attach_worker` stores the worker iff `_sessions.get(sid) is session` under the lock (else closes it), applied at every spawn site incl. the post-turn `_restart_slash_worker`. - Single idempotent teardown funnel: session.close, WS disconnect, the generous-TTL idle reaper, shutdown, and the WS grace-reap all reach `_close_session_by_id` → `_teardown_session`; `_finalized`/`_closed` flags make concurrent/double teardown a no-op. `_sessions_lock` upgraded to RLock. - uvicorn `ws_ping_interval/timeout=20s` so a half-open socket (reverse-proxy 524) becomes a `WebSocketDisconnect` and the C1 path runs. Plus two review-driven hardening fixes (mine): - `session.active_list` now skips `_finalized` sessions so the footer "N sessions" count reflects attachable sessions instead of only ever growing until restart (NousResearch#38950). Keys on `_finalized` only, NOT the stdio sentinel, so a standalone `hermes --tui` session stays visible. - `_schedule_ws_orphan_reap._reap` pops via `_close_session_by_id` (under `_sessions_lock`) instead of `_sessions.pop` under the unrelated `_session_resume_lock` (NousResearch#39591); the resume_lock now only guards the orphan re-check against `session.resume`. - Float env knobs (`HERMES_SLASH_WATCHDOG_*`, `HERMES_TUI_SESSION_TTL_S`) parse with a fallback helper so a malformed value can't crash the worker at import. Fixes NousResearch#32377 Fixes NousResearch#38950 Addresses NousResearch#22855 Co-authored-by: banditburai <123342691+banditburai@users.noreply.github.com> Co-authored-by: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com>

…ctive_list liveness (re-scoped onto current main) Salvaged from #35626 (banditburai) and re-scoped after maintainers landed the parent-death watchdog (slash_worker.py) and PTY process-group teardown (pty_bridge.py) directly on main. Those pieces are intentionally NOT included here — this carries only what is still missing: - C1 disconnect reap: ws.py's `finally` only re-pointed the dead transport at stdio. `_close_sessions_for_transport` now reaps `close_on_disconnect` sessions and schedules the grace-reap for the rest, offloaded via `asyncio.to_thread` so the blocking worker.close() + DB write never stalls the uvicorn loop. - C2 create/close orphan race: `_attach_worker` stores the worker iff `_sessions.get(sid) is session` under the lock (else closes it), applied at every spawn site incl. the post-turn `_restart_slash_worker`. - Single idempotent teardown funnel: session.close, WS disconnect, the generous-TTL idle reaper, shutdown, and the WS grace-reap all reach `_close_session_by_id` → `_teardown_session`; `_finalized`/`_closed` flags make concurrent/double teardown a no-op. `_sessions_lock` upgraded to RLock. - uvicorn `ws_ping_interval/timeout=20s` so a half-open socket (reverse-proxy 524) becomes a `WebSocketDisconnect` and the C1 path runs. Plus two review-driven hardening fixes (mine): - `session.active_list` now skips `_finalized` sessions so the footer "N sessions" count reflects attachable sessions instead of only ever growing until restart (#38950). Keys on `_finalized` only, NOT the stdio sentinel, so a standalone `hermes --tui` session stays visible. - `_schedule_ws_orphan_reap._reap` pops via `_close_session_by_id` (under `_sessions_lock`) instead of `_sessions.pop` under the unrelated `_session_resume_lock` (#39591); the resume_lock now only guards the orphan re-check against `session.resume`. - Float env knobs (`HERMES_SLASH_WATCHDOG_*`, `HERMES_TUI_SESSION_TTL_S`) parse with a fallback helper so a malformed value can't crash the worker at import. Fixes #32377 Fixes #38950 Addresses #22855 Co-authored-by: banditburai <123342691+banditburai@users.noreply.github.com> Co-authored-by: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com>

alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/gateway Gateway runner, session dispatch, delivery P1 High — major feature broken, no workaround labels May 31, 2026

alt-glitch mentioned this pull request May 31, 2026

fix(gateway): raise macOS launchd fd ceiling (8192/16384) + self-heal existing installs #35627

Open

tonydwb approved these changes May 31, 2026

View reviewed changes

banditburai added 13 commits June 1, 2026 13:09

fix(tui-gateway): harden _SlashWorker.close (reap zombie + close fds,…

5f8d8f8

… idempotent) Task: swl-qrf.2

refactor(tui-gateway): route session.close + shutdown through _close_…

fdcf512

…session_by_id Task: swl-qrf.4

feat(tui-gateway): close_on_disconnect flag + _close_sessions_for_tra…

8f60497

…nsport Task: swl-qrf.5

feat(dashboard): mark sidecar sessions close_on_disconnect

1d8e1dd

Task: swl-qrf.7

fix(dashboard): enable uvicorn WS ping to detect half-open connections

b1e39a0

Task: swl-qrf.11

banditburai force-pushed the fix/tui-gateway-slash-worker-leak branch from cfbe81c to 24000c4 Compare June 1, 2026 17:58

alt-glitch mentioned this pull request Jun 3, 2026

fix(web_server): reap finished action subprocesses to prevent zombie accumulation #38045

Closed

13 tasks

heshamkhaledd mentioned this pull request Jun 7, 2026

Dashboard hangs: tui_gateway.slash_worker subprocesses leak on PTY chat disconnect (524 via reverse proxy) #32377

Closed

teknium1 mentioned this pull request Jun 8, 2026

fix(tui/pty): reap leaked slash_worker subprocesses on PTY chat disconnect #42132

Merged

teknium1 closed this Jun 8, 2026

kshitijk4poor mentioned this pull request Jun 8, 2026

fix(tui-gateway): reap leaked slash_worker subprocesses on disconnect/create-race/parent-death + active_list liveness (#32377, #38950) #42143

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tui-gateway): reap leaked slash_worker processes/fds on disconnect, create-race, and parent-death#35626

fix(tui-gateway): reap leaked slash_worker processes/fds on disconnect, create-race, and parent-death#35626
banditburai wants to merge 13 commits into
NousResearch:mainfrom
banditburai:fix/tui-gateway-slash-worker-leak

banditburai commented May 31, 2026 •

edited

Loading

Uh oh!

alt-glitch commented May 31, 2026

Uh oh!

tonydwb left a comment

Uh oh!

teknium1 commented Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

banditburai commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Root cause → mechanism → file → commit

Cross-cutting note

Resolves

Known limitations

Tests

Reviewer guidance

Uh oh!

alt-glitch commented May 31, 2026

Uh oh!

tonydwb left a comment

Choose a reason for hiding this comment

Code Review Summary

Reviewed Changes

✅ Looks Good

Uh oh!

teknium1 commented Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

banditburai commented May 31, 2026 •

edited

Loading