fix(terminal): finalize sessions abandoned mid-handshake so slots aren't leaked - #579
Open
Farahat612 wants to merge 1 commit into
Open
fix(terminal): finalize sessions abandoned mid-handshake so slots aren't leaked#579Farahat612 wants to merge 1 commit into
Farahat612 wants to merge 1 commit into
Conversation
…n't leaked A browser refresh/close during WS open lands in onClose while onOpen is still awaiting the SSH channel and audit-row insert. teardown then ran with no sessionId, marked the connection ended, and onOpen resumed to register a session nobody owns: the idle/cap timeout's teardown became a no-op, leaving the terminal_sessions row open forever. Since the handshake cap counts DB rows with endedAt IS NULL, each abandoned open permanently burned a per-user slot until a restart's closeAllActive() sweep (oblien#426). teardown now defers to onOpen while no session is registered (leaving state.closed set as the abort signal), and onOpen aborts after registration when the WS is already gone: unregistering and finalizing the row instead of wiring a dead connection. Park/resume after "ready" is unchanged. Same-shape fix in the service-terminal controller. New regression test drives the real handler bundle with onClose landing mid-openShell: fails before (in-memory + DB counts stay 1), passes after; a sibling test pins that post-ready disconnects still park.
Farahat612
force-pushed
the
fix/426-terminal-session-leak
branch
from
August 16, 2026 11:43
d965339 to
313c87e
Compare
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.
A browser refresh/close during WS open lands in onClose while onOpen is
still awaiting the SSH channel and audit-row insert. teardown then ran
with no sessionId, marked the connection ended, and onOpen resumed to
register a session nobody owns: the idle/cap timeout's teardown became a
no-op, leaving the terminal_sessions row open forever. Since the
handshake cap counts DB rows with endedAt IS NULL, each abandoned open
permanently burned a per-user slot until a restart's closeAllActive()
sweep (#426).
teardown now defers to onOpen while no session is registered (leaving
state.closed set as the abort signal), and onOpen aborts after
registration when the WS is already gone: unregistering and finalizing
the row instead of wiring a dead connection. Park/resume after "ready"
is unchanged. Same-shape fix in the service-terminal controller.
New regression test drives the real handler bundle with onClose landing
mid-openShell: fails before (in-memory + DB counts stay 1), passes
after; a sibling test pins that post-ready disconnects still park.
Closes #426.
Root cause in one line: the WebSocket close listener is registered before
the async open handler resolves, so a browser refresh mid-handshake ran
teardown with no sessionId, marked the connection ended, and the session
registered moments later could never be finalized. The terminal_sessions row
(whose endedAt IS NULL count gates the 3-session cap) stayed open until a
restart's closeAllActive() sweep, which matches the reported "persists for
days, docker compose restart fixes it".
How verified: the new test drives the real handler bundle with onClose
landing while onOpen is suspended inside the SSH channel open. It fails on
main (in-memory and DB active counts stay 1) and passes with the fix. A
sibling test pins that a disconnect after the ready frame still parks the
session for resume, so the intentional park/resume design is unchanged. The
same-shape fix is applied to the service-terminal controller. Full apps/api
suite green (2993 tests), typecheck clean.