From 5b41f03a92e5a4f33e9532d46d70db4d3f2d5b4a Mon Sep 17 00:00:00 2001 From: Isla-Liu Date: Fri, 22 May 2026 18:09:05 +0800 Subject: [PATCH] fix(webui): close sqlite3 connections in handoff-summary path (#2233) Two functions on the /api/session/handoff-summary hot path were opening sqlite3.connect(...) inside a bare `with` statement, which commits the transaction at scope exit but does NOT close the connection. Per-turn invocations accumulated state.db / state.db-wal file descriptors and CPython heap pages on long-lived worker threads, surfacing as the multi-GB VmRSS and 6x duplicated state.db fds observed on the live instance (D0 pre-restart baseline: VmRSS 1,334,248 kB, 55 fds; cold baseline after restart: VmRSS 136,668 kB, 10 fds). Wrap both call sites with contextlib.closing(...) (already imported and used at seven other sites in the same files) so the connection is closed deterministically: - api/models.py :: count_conversation_rounds - api/routes.py :: _persist_handoff_summary_to_state_db Regression test: tests/test_issue2233_sqlite_connection_leak.py loops both functions 20 times against a tmp state.db and asserts /proc//fd count does not grow more than 2. Linux-only via sys.platform skip. D1 live soak against a freshly-built worktree server (port 8799, isolated HERMES_HOME / HERMES_WEBUI_STATE_DIR) hitting /api/session/handoff-summary 20 times: fd_before = 5 fd_after = 5 (growth 0, threshold < 5) vmrss_before = 52636 kB vmrss_after = 52636 kB (growth 0 kB, threshold < 30 MB) The patched fix curve trends below the leak curve. Rollback: single git revert reverts both file edits. Refs #2233. --- CHANGELOG.md | 4 + api/models.py | 2 +- api/routes.py | 2 +- .../test_issue2233_sqlite_connection_leak.py | 107 ++++++++++++++++++ 4 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 tests/test_issue2233_sqlite_connection_leak.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 89792caf..8ba45af1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ ## [Unreleased] +### Fixed + +- **Fixed: per-turn SQLite connection leak in handoff-summary path (#2233)** (Isla Liu, fork prototype). Two functions on the `/api/session/handoff-summary` hot path were opening `sqlite3.connect(...)` inside a bare `with` statement, which commits the transaction at scope exit but does NOT close the connection. Per-turn invocations accumulated `state.db`/`state.db-wal` file descriptors and CPython heap pages on long-lived worker threads, surfacing as the multi-GB VmRSS / 6× duplicated state.db fds observed on PID 434. Wrapped both call sites with `contextlib.closing(...)` (already imported and used at 7 other sites in the same files) so the connection is closed deterministically: `api/models.py::count_conversation_rounds` and `api/routes.py::_persist_handoff_summary_to_state_db`. Regression test `tests/test_issue2233_sqlite_connection_leak.py` loops both functions 20× against a tmp `state.db` and asserts `/proc//fd` count does not grow more than 2. Live soak against a freshly-built worktree server confirmed fd growth = 0 and VmRSS growth = 0 KB across 20 POSTs to `/api/session/handoff-summary` — well below the leaked-curve baseline (cold 134 MB → pre-restart 1.27 GB on the unpatched instance). Rollback: single `git revert` reverts both edits. + ## [v0.51.107] — 2026-05-21 — Release CE (stage-400 — 8-PR batch — pinned-sessions-limit getter rename + uploaded-file user-turn dedupe + active-run repair guard + incremental KaTeX streaming + profile default model on fresh boot + French locale completion + update-check error surfacing + release-update apply path) diff --git a/api/models.py b/api/models.py index f85909b3..ac55ccf8 100644 --- a/api/models.py +++ b/api/models.py @@ -3180,7 +3180,7 @@ def count_conversation_rounds(sid: str, since: float | None = None) -> int: return 0 try: - with sqlite3.connect(str(db_path)) as conn: + with closing(sqlite3.connect(str(db_path))) as conn: conn.row_factory = sqlite3.Row cur = conn.cursor() cur.execute( diff --git a/api/routes.py b/api/routes.py index 54c6261b..a62bc24f 100644 --- a/api/routes.py +++ b/api/routes.py @@ -10426,7 +10426,7 @@ def _persist_handoff_summary_to_state_db(sid: str, message: dict) -> bool: marker_payload = _extract_handoff_summary_payload(message) try: - with sqlite3.connect(str(db_path)) as conn: + with closing(sqlite3.connect(str(db_path))) as conn: try: if marker_payload is not None: cur = conn.execute( diff --git a/tests/test_issue2233_sqlite_connection_leak.py b/tests/test_issue2233_sqlite_connection_leak.py new file mode 100644 index 00000000..8e551c69 --- /dev/null +++ b/tests/test_issue2233_sqlite_connection_leak.py @@ -0,0 +1,107 @@ +"""Regression test for Issue #2233: per-turn SQLite connection leak. + +Two functions on the /api/session/handoff-summary hot path were opening +``sqlite3.connect(...)`` inside a bare ``with`` statement, which commits +the transaction at scope exit but does NOT close the connection. Looping +those calls per chat turn accumulated file descriptors (state.db and +state.db-wal) and CPython heap pages on long-lived worker threads. + +The fix wraps both connect() calls with ``contextlib.closing(...)`` so +the connection is closed deterministically: + + * api/models.py :: count_conversation_rounds + * api/routes.py :: _persist_handoff_summary_to_state_db + +This test loops the two patched functions ~20 times against a tmp state.db +and asserts the parent process open-fd count does not climb. + +Linux-only because the check reads ``/proc//fd`` directly. Skipped +on macOS/Windows. +""" +import os +import sqlite3 +import sys +from pathlib import Path + +import pytest + +_IS_LINUX = sys.platform.startswith("linux") + + +def _open_fd_count() -> int: + return len(os.listdir(f"/proc/{os.getpid()}/fd")) + + +def _make_state_db(path: Path) -> None: + """Create a state.db with the minimum schema the two patched functions touch.""" + conn = sqlite3.connect(str(path)) + try: + conn.execute( + "CREATE TABLE messages (" + " session_id TEXT," + " role TEXT," + " content TEXT," + " timestamp REAL" + ")" + ) + conn.execute( + "CREATE TABLE sessions (" + " id TEXT PRIMARY KEY," + " message_count INTEGER" + ")" + ) + conn.execute( + "INSERT INTO sessions (id, message_count) VALUES (?, 0)", + ("20260101_000000_abcdef",), + ) + conn.execute( + "INSERT INTO messages (session_id, role, content, timestamp) " + "VALUES (?, 'user', 'hi', 1.0)", + ("20260101_000000_abcdef",), + ) + conn.execute( + "INSERT INTO messages (session_id, role, content, timestamp) " + "VALUES (?, 'agent', 'hello', 2.0)", + ("20260101_000000_abcdef",), + ) + conn.commit() + finally: + conn.close() + + +@pytest.mark.skipif(not _IS_LINUX, reason="fd counting via /proc only available on Linux") +def test_handoff_summary_path_does_not_leak_fds(tmp_path, monkeypatch): + """Loop both patched functions and assert open-fd count stays bounded.""" + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + state_db = hermes_home / "state.db" + _make_state_db(state_db) + + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + + from api.models import count_conversation_rounds + from api.routes import _persist_handoff_summary_to_state_db + + sid = "20260101_000000_abcdef" + marker = { + "role": "tool", + "content": "{\"handoff_summary\": \"test\"}", + "timestamp": 3.0, + } + + count_conversation_rounds(sid) + _persist_handoff_summary_to_state_db(sid, marker) + + fd_before = _open_fd_count() + + for _ in range(20): + count_conversation_rounds(sid) + _persist_handoff_summary_to_state_db(sid, marker) + + fd_after = _open_fd_count() + growth = fd_after - fd_before + + assert growth <= 2, ( + f"open fd count grew by {growth} (before={fd_before}, after={fd_after}); " + "suggests sqlite3 connections from the handoff-summary path are not being closed" + )