fix(dashboard): reap orphaned embedded-chat sessions to stop slash_worker leak - #39502
Merged
Conversation
…rker leak Since #38591 made the dashboard's embedded chat unconditional, every browser refresh of /chat spins up a fresh session.create (new sid + a fresh _SlashWorker via _deferred_build) over /api/ws, but the old tab's WS disconnect only DETACHES the transport (ws.py) — it never closes the old session or its slash_worker. The dashboard's in-process gateway is long-lived, so the detached _SlashWorker subprocess's stdin pipe stays open forever and the worker never reaches EOF: one leaked python process per refresh. Fix at the session-lifecycle layer (not PTY signal timing — verified that a process whose owning gateway dies is always reaped via stdin-EOF; the leak is specifically the long-lived dashboard process keeping detached sessions parked). On WS disconnect, schedule a grace-delayed reap of any session left orphaned (transport detached to stdio, not mid-turn). A quick reconnect / session.resume / prompt.submit rebinds a live transport and cancels the reap, preserving the intentional detach-for-reconnect window. - server.py: extract _teardown_session() (shared with session.close), add _ws_session_is_orphaned() + _schedule_ws_orphan_reap(), gated by HERMES_TUI_WS_ORPHAN_REAP_GRACE_S (default 20s, 0 disables). - ws.py: schedule the reap for each detached session on disconnect. - tests: reap-closes-worker, spares-reattached/mid-turn/finalized, disabled-when-grace-zero.
Contributor
🔎 Lint report:
|
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.
Summary
Since #38591 made the dashboard's embedded Chat surface unconditional, every browser refresh of
/chatleaks atui_gateway.slash_workerPython subprocess — they accumulate one-per-refresh until the box is buried in idle workers (user report: ~20 workers after a session of refreshing).Root cause
The dashboard runs one long-lived in-process
tui_gateway(the PTY child node attaches to it over/api/wsviaHERMES_TUI_GATEWAY_URLrather than spawning its own Python gateway — see_resolve_chat_argv). So the topology is:On every refresh the fresh Ink boot calls
session.create(new sid + a fresh_SlashWorkervia_deferred_build). The old tab's/api/wsdisconnect, however, only detaches the transport intui_gateway/ws.py(sess["transport"] = _stdio_transport) to allow reconnects — it never closes the old session or its slash worker. Because the dashboard process never exits, the detached_SlashWorker.proc.stdinwrite-end stays open forever, the worker never hits EOF, and it leaks.This was dormant before #38591 because the whole
/api/ws+session.create+_SlashWorkerpath was gated off on a plain dashboard.Note: this is not a PTY signal-timing / atexit problem. Verified empirically that a worker whose owning process dies (even via
os._exit(0)orSIGKILL) is always reaped via stdin-EOF; the leak is specifically the long-lived dashboard keeping detached sessions parked indefinitely.Fix
Reap orphaned embedded-chat sessions at the session-lifecycle layer. On WS disconnect, schedule a grace-delayed reap of any session left orphaned (transport detached to stdio, not mid-turn). A quick reconnect /
session.resume/prompt.submitrebinds a live transport and cancels the reap, preserving the intentional detach-for-reconnect window.tui_gateway/server.py: extract_teardown_session()(shared withsession.close), add_ws_session_is_orphaned()+_schedule_ws_orphan_reap(). Gated byHERMES_TUI_WS_ORPHAN_REAP_GRACE_S(default 20s;0disables = pre-fix park-forever behaviour).tui_gateway/ws.py: schedule the reap for each detached session on disconnect.Testing
tests/test_tui_gateway_server.py+tests/tui_gateway/(305) +tests/gateway/test_ws_auth_retry.py+tests/hermes_cli/test_web_server.py(234) all green.tui_gateway.ws.handle_wsthrough 6 simulated refreshes against the real in-process gateway (stubbed_make_agentso the real_SlashWorkersubprocess still spawns).grace=0): 6 refreshes → 6 leaked workers, none reaped.grace=3): only 1 worker alive at any time during the refreshes, 0 remaining after the grace window.hermes dashboardfrom this branch, refreshed/chatrepeatedly while watchingpgrep -f tui_gateway.slash_worker— count bumps to 2 briefly after a refresh and settles back to 1 within the grace window; no accumulation. Confirmed working.Review note
Touches
tui_gateway/, so this needs @teknium1 review (outside the Docker lane).Fixes the slash-worker accumulation introduced as fallout from #38591.