fix(tui_gateway): close dedicated profile SessionDB handles on every path (salvage #76701) - #81071
Merged
Merged
Conversation
Follow-up to the review on the session.resume ownership fix. Closing the
pre-transfer early returns left two gaps, both real.
1. The transfer had no owner on the other side. Once ownership moved to the
agent, teardown ran AIAgent.close() (via _teardown_session on session.close
and the orphaned-session reaper), which called session_db.end_session() —
that finalizes the session ROW, not the connection. A successfully resumed
profile session kept its dedicated handle, its db/-wal/-shm fds and its
background token-writer thread for the life of the gateway.
AIAgent now carries an explicit _owns_session_db, defaulting False so the
SHARED launch handle — which outlives every agent and backs every other live
session — is still never closed there. Only the dedicated-open sites set it,
at the point ownership actually changes hands.
2. session.resume was not the only profile-scoped open with no close on its
failure paths. Covered here with the same flag, via a _transfer_db_to_agent
helper that refuses the transfer unless the agent really holds that handle:
- the deferred builder (_start_agent_build), including the session-reaped-
mid-build case, where the built agent is discarded and never torn down, so
transferring to it would leak exactly as before;
- session.branch's branch_db;
- the compute host's per-profile open;
- AIAgent's own lazy open in _get_session_db_for_recall, which no other
object ever references and so was unconditionally abandoned.
Where a handle has already reached a registered session, the drop is
unconditional and the transfer is best-effort on top: a refused transfer leaves
the old leak, which is survivable, whereas closing under a live session is the
permanent "Cannot operate on a closed database" break the original patch exists
to avoid.
Tests: tests/tui_gateway/test_session_db_ownership_teardown.py (new, 14).
11 of the 14 fail without this change; the 3 that pass are the "must NOT close"
guards, which hold in both directions by design.
Review finding: the close block's comment promises a raising session_db.close() is swallowed with the flag already cleared (no re-close on a second agent.close()), but nothing pinned it. One test with a raising _RecordingDB proves both halves.
This was referenced Aug 7, 2026
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
Dedicated profile-scoped
SessionDBhandles are now closed on every path — the pre-transfer early returns insession.resume, the deferred/branch/compute-host build failure paths, the reaped-mid-build discard, and agent teardown (AIAgent.close()releases a handle it owns). Fixes the fd leak class where long-lived Desktop/servebackends accumulated 2 fds + a pinned token-writer thread per abandoned handle untilOSError: [Errno 24] Too many open files.Salvage of #76701 by @Yishova — both commits cherry-picked with authorship preserved. Supersedes #80706 (strict subset: teardown half only, tests break on macOS), overlaps #78970/#72804 at other layers.
Who hits this
Anyone running the Desktop app or
hermes serveagainst multiple profiles for days: every reconnect/tile-paint resume of a live chat (session.resumefast path — the hot one), every default cold resume, and every successfully-built-then-closed agent leaked a SQLite handle. An abandonedSessionDBis never GC'd once its background token writer starts — it pins ITSELF viaatexit.register(_drain_token_queue_at_exit), which onlyclose()unregisters. Observed live: ~490 leaked fds over ~2 days, backend wedged at the 1024-fd soft limit.Changes
tui_gateway/methods_session.py—session.resumegets explicitowns_dbownership with atry/finallyclose covering all ten pre-transfer early returns; ownership transfers exactly at_init_sessionsuccess; half-built session registration rolled back on failure (prevents a permanently-dead live-session fast path).agent/agent_init.py+run_agent.py—_owns_session_dblifecycle flag (mirrors_end_session_on_close); the recall fallback's self-created handle is owned;AIAgent.close()releases an owned handle afterend_session(), clearing the flag first for idempotency.tui_gateway/server.py+compute_host.py— deferred builder, branch handler, and compute-host paths transfer ownership via_transfer_db_to_agent()(refuses mismatched handles; closes directly when the built agent was discarded mid-build)._RecordingDBdoubles — owned-closed-exactly-once, shared-never-closed, idempotency, transfer refusal, reaped-mid-build, registration rollback,end_session→closeordering.Note the raw diff on
methods_session.pyis mostly re-indentation from thetry:wrap; review withgit diff -w(~40 insertions / 2 deletions there).Validation
session.resume(whitespace-normalized hunk set verified identical to the original)Related PRs in this cluster
_owns_session_dbteardown mechanism, opened 5 days later; subset of this PR (misses all pre-transfer paths, marks the reaped-mid-build agent owner without anything closing it,/proc/self/fdtests hard-fail on macOS). Recommend closing with credit once this merges._teardown_sessiononly; fix: close SessionDB FDs on timeout and lazy recall paths (#72782) #72804 — timeout + lazy-recall paths only; fix(state): close threaded SessionDB read conns without leaking FDs #74304/fix(hermes_state): bound per-thread read-connection cache to prevent fd leak #75546/state: pool SessionDB read connections instead of leaking one per (SessionDB x thread) #76700 — per-thread READ connection cache (different leak, separate fix).Based on #76701 by @Yishova — commits cherry-picked to preserve authorship.