fix(tui): close slash_worker inside _finalize_session (defense-in-depth, #38095)#42149
Merged
Conversation
…th, #38095) Fold the slash-worker subprocess close into _finalize_session itself — the single _finalized-guarded session-end chokepoint — instead of relying on each caller (_teardown_session, _shutdown_sessions) to close it separately. A future code path that finalizes a session directly can no longer reintroduce the #38095 worker leak. Idempotent: _SlashWorker.close() is poll()-guarded and _finalize_session short-circuits on _finalized, so the existing teardown paths are unaffected. Drops the now-redundant separate close() in _shutdown_sessions. Note: the active leak this issue reported was already fixed on main (WS-orphan reaper #38591, _restart_slash_worker close, atexit shutdown). This addresses the residual defense-in-depth gap the reporter correctly identified in their follow-up comment.
Contributor
🔎 Lint report:
|
jhjaggars-hermes
pushed a commit
to jhjaggars/hermes-agent
that referenced
this pull request
Jun 8, 2026
…th, NousResearch#38095) (NousResearch#42149) Fold the slash-worker subprocess close into _finalize_session itself — the single _finalized-guarded session-end chokepoint — instead of relying on each caller (_teardown_session, _shutdown_sessions) to close it separately. A future code path that finalizes a session directly can no longer reintroduce the NousResearch#38095 worker leak. Idempotent: _SlashWorker.close() is poll()-guarded and _finalize_session short-circuits on _finalized, so the existing teardown paths are unaffected. Drops the now-redundant separate close() in _shutdown_sessions. Note: the active leak this issue reported was already fixed on main (WS-orphan reaper NousResearch#38591, _restart_slash_worker close, atexit shutdown). This addresses the residual defense-in-depth gap the reporter correctly identified in their follow-up comment.
a249169329-cpu
pushed a commit
to a249169329-cpu/hermes-agent
that referenced
this pull request
Jun 9, 2026
…th, NousResearch#38095) (NousResearch#42149) Fold the slash-worker subprocess close into _finalize_session itself — the single _finalized-guarded session-end chokepoint — instead of relying on each caller (_teardown_session, _shutdown_sessions) to close it separately. A future code path that finalizes a session directly can no longer reintroduce the NousResearch#38095 worker leak. Idempotent: _SlashWorker.close() is poll()-guarded and _finalize_session short-circuits on _finalized, so the existing teardown paths are unaffected. Drops the now-redundant separate close() in _shutdown_sessions. Note: the active leak this issue reported was already fixed on main (WS-orphan reaper NousResearch#38591, _restart_slash_worker close, atexit shutdown). This addresses the residual defense-in-depth gap the reporter correctly identified in their follow-up comment.
changman
pushed a commit
to changman/hermes-agent
that referenced
this pull request
Jun 10, 2026
…th, NousResearch#38095) (NousResearch#42149) Fold the slash-worker subprocess close into _finalize_session itself — the single _finalized-guarded session-end chokepoint — instead of relying on each caller (_teardown_session, _shutdown_sessions) to close it separately. A future code path that finalizes a session directly can no longer reintroduce the NousResearch#38095 worker leak. Idempotent: _SlashWorker.close() is poll()-guarded and _finalize_session short-circuits on _finalized, so the existing teardown paths are unaffected. Drops the now-redundant separate close() in _shutdown_sessions. Note: the active leak this issue reported was already fixed on main (WS-orphan reaper NousResearch#38591, _restart_slash_worker close, atexit shutdown). This addresses the residual defense-in-depth gap the reporter correctly identified in their follow-up comment.
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
1 participant
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
Closing the TUI slash-worker subprocess is now a side effect of
_finalize_sessionitself — the single_finalized-guarded session-end chokepoint — so no session-end path can leak the worker.The active leak #38095 reported was already fixed on
main(WS-orphan reaper from #38591,_restart_slash_workerclosing the old worker, atexit_shutdown_sessions). This addresses the residual defense-in-depth gap the reporter correctly identified in their follow-up comment: worker cleanup lived only in the callers, so a future code path that calls_finalize_session()directly would silently re-leak.Changes
tui_gateway/server.py:_finalize_session()now closessession["slash_worker"]as its last step. Updated_teardown_sessiondocstring; dropped the now-redundant separateworker.close()in_shutdown_sessions.tests/test_tui_gateway_server.py: regression test —_finalize_sessioncloses the worker exactly once and is idempotent across repeat finalize + follow-up teardown.Idempotent by construction:
_SlashWorker.close()ispoll()-guarded and_finalize_sessionshort-circuits on_finalized, so existing teardown paths (session.close, WS-orphan reap, shutdown) behave identically.Validation
_finalize_sessiondirect callsession.close/ WS reap / shutdowntests/test_tui_gateway_server.pyCloses #38095.
Infographic