Skip to content

fix(state): bound session database file descriptors - #79457

Open
jg-visture wants to merge 1 commit into
NousResearch:mainfrom
jg-visture:fix/session-db-fd-leaks
Open

fix(state): bound session database file descriptors#79457
jg-visture wants to merge 1 commit into
NousResearch:mainfrom
jg-visture:fix/session-db-fd-leaks

Conversation

@jg-visture

Copy link
Copy Markdown

Summary

  • bound each SessionDB to one synchronized read-only WAL connection instead of retaining one connection per transient reader thread
  • close reaction-tool-owned SessionDB instances on every return path
  • reuse the caller-owned execution database for registered reaction calls
  • add regressions for short-lived reader-thread churn and reaction database ownership

Root cause

A long-running macOS gateway reached its 256-descriptor soft limit with 88 state.db and 86 state.db-wal handles. The transient gateway reader threads were retained through per-thread SQLite connections, and standalone reaction calls created SessionDB instances without deterministic closure. Descriptor exhaustion then surfaced as provider connection failures (OSError: [Errno 24] Too many open files followed by openai.APIConnectionError).

Verification

  • uv run --extra dev pytest -q tests/test_session_db_read_path_split.py tests/test_message_reactions.py — 19 passed, 4 skipped
  • uv run --extra dev ruff check hermes_state.py tools/react_to_message_tool.py tests/test_session_db_read_path_split.py tests/test_message_reactions.py — passed
  • pre-rebase relevant suite — 189 passed, 9 skipped
  • live patched gateway descriptors remained bounded around 23–39 total FDs and 0–12 database handles during observation, versus 255 total before repair
  • real provider smoke test returned the expected response after restart

Notes

The shared read connection has check_same_thread=False and is serialized under an independent RLock, preserving read/write separation while bounding descriptors. Caller-owned reaction databases are never closed by the tool.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state needs-decision Awaiting maintainer decision before any implementation labels Aug 5, 2026
@MongLong0214

Copy link
Copy Markdown

The shared-reader serialization looks sound, but two failure paths still violate the FD-lifecycle guarantee this PR is meant to establish.

1. Partial reader initialization loses the candidate

_get_read_conn() constructs conn, then runs apply_database_pragmas() and optionally load_fts5_cjk_extension() before publishing it to self._read_conn. If either initialization step fails:

  • a sqlite3.Error is caught and the method returns None without closing conn;
  • a non-sqlite3.Error escapes, also without an explicit rollback close.

The connection has already been physically opened at that point. Falling back to the writer does not retire that candidate.

2. Failed close clears the final owner

close() catches an exception from self._read_conn.close() and then unconditionally sets self._read_conn = None. A failed physical close therefore loses the last retry/quarantine owner while the descriptor may still be live.

Please add deterministic regressions for:

  • test_shared_reader_partial_init_closes_candidate_before_writer_fallback
  • test_shared_reader_partial_init_close_failure_retains_retry_owner
  • test_session_db_close_failure_does_not_clear_shared_reader_owner

The construction invariant should be: reserve/construct, complete every initialization step, then publish; on any intermediate failure, close the candidate before returning the reservation. If that rollback close itself fails, retain a strong failed-close owner rather than marking the reader absent. The same physical-close rule should apply during SessionDB.close().

Without these paths, normal operation is bounded to one reader, but repeated initialization failures or a close failure can still accumulate unowned descriptors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants