You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Fixes the SQLite connection leak on state.db that slowly exhausts a
long-lived gateway's fd table (#96027 — 48 state.db + 46 state.db-wal
fds after 22 days, then [Errno 24] Too many open files everywhere).
The leak class is read paths that open a fresh SessionDB per call site
and never close it. Audited and fixed/verified:
tools/session_search_tool.py — the default SessionDB() opened when no db is passed, and the cross-profile read-only handle, are now closed by
the caller (owned-handle contract). Verified against the v0.20.0 code: 10
tool calls leaked 2 connections each; the fixed code leaks 0.
tools/react_to_message_tool.py — handle now closed on every return path.
RecoverableHandleCache (the gateway's per-path SessionDB cache) — a
handle that loses the caching race (concurrent close_all() /
unavailable-entry replacement mid-open) was dropped without close(),
stranding its state.db fds for the life of the process. It is now always
released, even before any close_all() callback is installed.
The v0.20.0-era readiness probe (gateway/readiness.py) and
session_search ownership fixes are already in main; this PR pins them with
regression tests.
Leak-visibility guard (the issue's ask)
hermes_cli/sqlite_safe_read.py gains a public live_connection_count(path)
helper (thread-safe read of the tracked-connection registry — the cheap,
cross-platform analogue of counting /proc/self/fd entries). Gateway
housekeeping (gateway/run.py) now calls it hourly:
INFO whenever the live connection count for state.db grows since the
previous tick (the ~2 connections/day signature becomes visible within
hours instead of at EMFILE);
WARNING once the count exceeds the healthy ceiling
(2 long-lived SessionDBs × 1 writer + 8 pooled readers + margin = 22).
open/close pairing with a pinned-connection model (no GC escape hatch, the
way production's atexit pin behaves) for session_search default,
cross-profile, and react_to_message — red on the v0.20.0 tool code
(leaked 2/2/1 handles), green on the fixed code;
live-connection-count invariants: gateway turns and SessionDB read loops
stay flat after warm-up; close()/close_all_db_handles() drain to zero;
RecoverableHandleCache rejected-handle contract — red without the
fix ("rejected handle was dropped without close()"), green with it;
live_connection_count + the housekeeping growth guard.
tests/gateway + tests/hermes_state + state-db repair suites: all green
except test_73771_media_resend_dedup.py::test_streamed_explicit_media_resend_is_delivered,
which fails identically on clean main (pre-existing, unrelated)
AI code review — automated review for reference, author can ignore or act on any point.
This PR closes the SQLite connection leak on a long-lived gateway's state.db (#96027). The core behavioral fix is in the gateway's per-path handle cache: a handle that loses the caching race (concurrent close_all() / mid-open unavailable-entry replacement) was dropped without close(), stranding its fds for the process lifetime — it is now always released, falling back to the handle's own close() when no close_all callback is installed (gateway/session_db_recovery.py:167). It also adds a thread-safe live_connection_count() registry read (hermes_cli/sqlite_safe_read.py:149) and an hourly housekeeping fd-growth guard (gateway/run.py:30556), plus a 363-line regression test file that pins the open/close pairing with a pinned-connection model (no GC escape hatch, mirroring the production atexit pin).
Points:
(minor) The healthy ceiling is hardcoded as _FD_GUARD_WARN_CEILING = 2 * (1 + 8) + 4 (gateway/run.py:30553). The 8 is _READ_POOL_MAX (hermes_state.py:358). If that pool constant changes, the ceiling silently drifts from what a healthy gateway actually holds, so the guard could start false-warning or stop warning. Deriving the ceiling from _READ_POOL_MAX (or at least pointing the comment at it) would keep the heuristic honest. It is a WARNING heuristic, not a hard limit, so this is low severity.
(minor) The rejection-path closer is wrapped in except Exception: pass, so a close() that raises (e.g. a transient SQLite error) silently re-introduces exactly the leak this PR fixes — the handle's fd stays open with no diagnostic. The swallow is deliberate and covered by test_rejected_handle_close_failure_is_swallowed, but since the goal here is leak elimination, logging at debug/warning when a rejection-path close fails would keep a failing close visible instead of converting it into the same silent leak.
(observation) The growth guard logs INFO on any tick-to-tick growth and WARNING above the ceiling, but a leak that stabilizes between baseline and ceiling (e.g. settles at ~15) is never warned about once it stops growing — the hourly INFO stops firing and 15 < 22 so no WARNING. By design (early growth signal + late ceiling signal), so not a bug, but worth knowing that a plateaued leak below the ceiling is invisible to this guard.
Verified: key consistency between the guard's probe path and the registry keys holds — connect_tracked registers under _key(...) (resolved path), and the guard passes str(Path(_default_db_path()).resolve()), so counts match. The regression tests are strong: they model the production atexit pin (no GC escape), which is exactly why the v0.20.0 leak survived to EMFILE. The one mild brittleness is test_gateway_turns_keep_live_connection_count_flat's candidates-list heuristic for locating the store's db path — if SessionStore ever resolves elsewhere it fails at assert target is not None, but it's a reasonable approach for the conftest-pinned home.
Rebased onto origin/main (87ff1c2f7739); this PR is no longer 14,179 commits behind and git rev-list --count HEAD..origin/main is now 0.
Conflict resolution (2 files):
gateway/session_db_recovery.py — main had refactored the rejection branch to contextlib.suppress(Exception); kept that style with the new _close_handle fallback.
gateway/run.py — main had replaced the inline housekeeping tick with the chores list + _housekeeping_chore wrapper (Bug: gateway event loop stops silently while gateway_state.json still reports "running" — housekeeping, cron and embedded kanban dispatcher all freeze #113372). The fd guard now ships as one chore entry, (60, "state.db fd-leak guard", _housekeeping_state_db_fd_guard), instead of an inline if tick_count % FD_GUARD_EVERY block. Also changed _check_state_db_fd_growth(db_path) to _housekeeping_state_db_fd_guard() (resolves its own path) and replaced the hardcoded 2 * (1 + 8) + 4 ceiling with _fd_guard_warn_ceiling(), which imports _READ_POOL_MAX at call time so the ceiling cannot drift when the pool constant changes (the earlier AI review's only point).
Real-machine evidence (Windows, GetProcessHandleCount + the tracked-connection registry), 200 rejected opens, every opened SessionDB strongly referenced so GC cannot rescue it:
mode
live state.db connections
kernel handles
pre-fix (dropped without close)
200
+1602
this branch
0
+0 after GC
Tests: tests/gateway/test_state_db_fd_leak.py 9/9 green; the neighbouring gateway/state suites (test_session_db_recovery, test_session_db_handle_sharing, test_session_db_warning_recheck, test_session_db_leak_sweep, test_session_db_context_manager, test_session_db_read_conn_pool, test_sqlite_safe_read) all pass. test_shared_session_db_registry.py has 5 failures, but they are identical on clean origin/main (Windows WinError 32 unlinking a state.db that SQLite still holds open) — pre-existing, not from this change.
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
area/sessionsSession lifecycle, resume, persistence, historycomp/cliCLI entry point, hermes_cli/, setup wizardcomp/gatewayGateway runner, session dispatch, deliveryP2Medium — degraded but workaround existssweeper:risk-message-deliverySweeper risk: may drop, duplicate, misroute, or suppress messagessweeper:risk-session-stateSweeper risk: may lose/corrupt/mis-associate session or context statetype/bugSomething isn't working
3 participants
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
Fixes the SQLite connection leak on
state.dbthat slowly exhausts along-lived gateway's fd table (#96027 — 48
state.db+ 46state.db-walfds after 22 days, then
[Errno 24] Too many open fileseverywhere).The leak class is read paths that open a fresh
SessionDBper call siteand never close it. Audited and fixed/verified:
tools/session_search_tool.py— the defaultSessionDB()opened when nodbis passed, and the cross-profile read-only handle, are now closed bythe caller (owned-handle contract). Verified against the v0.20.0 code: 10
tool calls leaked 2 connections each; the fixed code leaks 0.
tools/react_to_message_tool.py— handle now closed on every return path.RecoverableHandleCache(the gateway's per-path SessionDB cache) — ahandle that loses the caching race (concurrent
close_all()/unavailable-entry replacement mid-open) was dropped without close(),
stranding its
state.dbfds for the life of the process. It is now alwaysreleased, even before any
close_all()callback is installed.gateway/readiness.py) andsession_search ownership fixes are already in main; this PR pins them with
regression tests.
Leak-visibility guard (the issue's ask)
hermes_cli/sqlite_safe_read.pygains a publiclive_connection_count(path)helper (thread-safe read of the tracked-connection registry — the cheap,
cross-platform analogue of counting
/proc/self/fdentries). Gatewayhousekeeping (
gateway/run.py) now calls it hourly:state.dbgrows since theprevious tick (the
~2 connections/daysignature becomes visible withinhours instead of at EMFILE);
(2 long-lived SessionDBs × 1 writer + 8 pooled readers + margin = 22).
Regression tests (
tests/gateway/test_state_db_fd_leak.py, 9 tests)way production's atexit pin behaves) for
session_searchdefault,cross-profile, and
react_to_message— red on the v0.20.0 tool code(leaked 2/2/1 handles), green on the fixed code;
stay flat after warm-up;
close()/close_all_db_handles()drain to zero;RecoverableHandleCacherejected-handle contract — red without thefix ("rejected handle was dropped without close()"), green with it;
live_connection_count+ the housekeeping growth guard.Test results
tests/gateway/test_state_db_fd_leak.py: 9 passedtests/gateway/test_session_db_recovery.py+tests/tools/test_session_search.pytests/hermes_state/test_session_read_state.py: 67 passedtests/gateway+tests/hermes_state+ state-db repair suites: all greenexcept
test_73771_media_resend_dedup.py::test_streamed_explicit_media_resend_is_delivered,which fails identically on clean main (pre-existing, unrelated)
ruff checkclean on all touched filesfd counts before/after
session_search(query=...)session_search(profile=...)react_to_message_toolCloses #96027