fix: long-lived processes stop minting duplicate state.db writer handles
Gateway, dashboard, ACP server and the CLI already hold one registry-shared SessionDB per state.db path, yet several in-process call paths still opened a bare SessionDB() beside it. Each one is a full writer: schema init, write lock, token-writer thread and a close-time WAL checkpoint. On a dashboard serving overlapping requests that stacked up to the "5 live SessionDB handles" precursor within seconds; per #110544 they are now harmless to each other's WAL generation, but the leak itself remained. Pure readers attach read_only=True (no writer connection, no write lock): - plugins/hermes-achievements/dashboard/plugin_api.py::scan_sessions (dashboard, per background scan and per /rescan; highest-frequency site) - hermes_cli/console_engine.py::_session_db (dashboard console; list, stats and export are reads; rename/optimize opt in to a writer) - tools/process_registry_results.py::_owns_result (gateway, per retained result load) - hermes_cli/main.py::_session_db (last-session / title / cwd lookups) - hermes_cli/terminal_breadcrumbs.py, hermes_cli/status.py, hermes_cli/main_tui_launch.py (lookup one-shots) Writers share the process's registry handle (hermes_state_registry.acquire; close()/release_or_close release one refcount): - acp_adapter/session.py::SessionManager._get_db — the AIAgent it builds acquires the same path, so the ACP server held two writers per process - hermes_cli/kanban_db_dispatch.py::_retag_legacy_worker_sessions (gateway dispatcher tick) - hermes_cli/main.py::_create_titled_session, hermes_cli/oneshot.py, hermes_cli/foreign_sessions.py — the CLI acquires the same handle a moment later The "N live SessionDB handles" warning now counts only writable members: read-only attaches are the sanctioned per-request shape for dashboard routers and CLI lookups, and counting them turned a healthy topology into an operator alarm (#100896 field reports of restart loops keyed on it). Live repro (one registry writer + 4 overlapping dashboard/gateway paths in one process): before 5 writable opens, 5 live handles, warning fired; after 1 writable open (the registry handle), 0 from the request paths, no warning. Refs #100896 #103339
This commit is contained in:
@@ -269,13 +269,15 @@ class SessionManager:
|
||||
return state
|
||||
|
||||
def _get_db(self):
|
||||
"""Lazily initialise the SessionDB; ``None`` if unavailable (e.g. import error in a
|
||||
minimal test env). ``HERMES_HOME`` is resolved here, not via the import-time
|
||||
``DEFAULT_DB_PATH``, so test fixtures that change the env var later are honoured."""
|
||||
"""Lazily acquire the process-shared SessionDB; ``None`` if unavailable (e.g. import
|
||||
error in a minimal test env). ``HERMES_HOME`` is resolved here, not via the import-time
|
||||
``DEFAULT_DB_PATH``, so test fixtures that change the env var later are honoured. The
|
||||
registry handle is the one in-process tools (delegation, session_search, goals) also
|
||||
acquire, so the ACP server holds ONE writer on state.db instead of two (#100896)."""
|
||||
if self._db_instance is None:
|
||||
try:
|
||||
from hermes_state import SessionDB
|
||||
self._db_instance = SessionDB(db_path=get_hermes_home() / "state.db")
|
||||
from hermes_state_registry import acquire
|
||||
self._db_instance = acquire(get_hermes_home() / "state.db")
|
||||
except Exception:
|
||||
logger.debug("SessionDB unavailable for ACP persistence", exc_info=True)
|
||||
return self._db_instance
|
||||
|
||||
@@ -584,10 +584,15 @@ def _logs(_engine: HermesConsoleEngine, args: list[str]) -> str:
|
||||
session=ns.session, since=ns.since, component=ns.component))
|
||||
|
||||
|
||||
def _session_db():
|
||||
"""``with _session_db() as db:`` — SessionDB closed on exit."""
|
||||
def _session_db(*, read_only: bool = True):
|
||||
"""``with _session_db() as db:`` — SessionDB closed on exit.
|
||||
|
||||
The console runs inside the dashboard process, which already owns a handle on state.db;
|
||||
reads attach ``read_only`` so they never add a writer connection beside it (#100896).
|
||||
Mutating commands pass ``read_only=False``.
|
||||
"""
|
||||
from hermes_state import SessionDB
|
||||
return closing(SessionDB())
|
||||
return closing(SessionDB(read_only=read_only))
|
||||
|
||||
|
||||
def _sessions_list(_engine: HermesConsoleEngine, args: list[str]) -> str:
|
||||
@@ -689,7 +694,7 @@ def _sessions_export(_engine: HermesConsoleEngine, args: list[str]) -> None:
|
||||
@_captured
|
||||
def _sessions_rename(_engine: HermesConsoleEngine, args: list[str]) -> None:
|
||||
ns = _parse("sessions rename", args, "session_id", (("title",), dict(nargs="+")))
|
||||
with _session_db() as db:
|
||||
with _session_db(read_only=False) as db:
|
||||
resolved_session_id = db.resolve_session_id(ns.session_id)
|
||||
title = " ".join(ns.title)
|
||||
if not resolved_session_id or not db.set_session_title(resolved_session_id, title):
|
||||
@@ -700,7 +705,7 @@ def _sessions_rename(_engine: HermesConsoleEngine, args: list[str]) -> None:
|
||||
@_captured
|
||||
def _sessions_optimize(_engine: HermesConsoleEngine, args: list[str]) -> None:
|
||||
_expect_no_args(args, "sessions optimize")
|
||||
with _session_db() as db:
|
||||
with _session_db(read_only=False) as db:
|
||||
print(f"Optimized {db.vacuum()} FTS index(es).")
|
||||
|
||||
|
||||
|
||||
@@ -242,8 +242,8 @@ def import_foreign_session(source: str, path, db=None) -> str:
|
||||
tool = _SOURCE_DB_NAMES[source]
|
||||
owns_db = db is None
|
||||
if owns_db:
|
||||
from hermes_state import SessionDB
|
||||
db = SessionDB()
|
||||
from hermes_state_registry import acquire
|
||||
db = acquire() # the CLI resume that follows acquires this same handle
|
||||
try:
|
||||
session_id = new_session_id()
|
||||
origin = {"imported_from": {"tool": tool, "path": str(path), "foreign_session_id": parsed.get("session_id")}}
|
||||
|
||||
@@ -2080,13 +2080,15 @@ def _retag_legacy_worker_sessions(workspaces_root_path: str) -> None:
|
||||
if workspaces_root_path in _retagged_workspace_roots:
|
||||
return
|
||||
try:
|
||||
from hermes_state import SessionDB
|
||||
from hermes_state_registry import acquire, release_or_close
|
||||
|
||||
db = SessionDB()
|
||||
# Inside the gateway the dispatcher shares the process's registry handle; a bare
|
||||
# SessionDB() here was one more writer connection on the same state.db (#100896).
|
||||
db = acquire()
|
||||
try:
|
||||
db.retag_kanban_worker_sessions(workspaces_root_path)
|
||||
finally:
|
||||
db.close()
|
||||
release_or_close(db)
|
||||
_retagged_workspace_roots.add(workspaces_root_path)
|
||||
except Exception as exc:
|
||||
_kb._log.debug("kanban worker: legacy session retag skipped (%s)", exc)
|
||||
|
||||
+10
-6
@@ -1157,14 +1157,16 @@ def _resolve_workspace_key() -> Optional[str]:
|
||||
|
||||
@contextlib.contextmanager
|
||||
def _session_db():
|
||||
"""Yield a ``SessionDB`` (lazy import, so test patches on ``hermes_state``
|
||||
intercept). Open failures yield None and any error raised by the ``with``
|
||||
body is swallowed — callers fall through to their ``return None``."""
|
||||
"""Yield a read-only ``SessionDB`` (lazy import, so test patches on ``hermes_state``
|
||||
intercept). Every caller is a lookup (last session, title → id, recorded cwd), so it
|
||||
never opens a writer beside the one the CLI acquires from the registry a moment later.
|
||||
Open failures yield None and any error raised by the ``with`` body is swallowed —
|
||||
callers fall through to their ``return None``."""
|
||||
db = None
|
||||
try:
|
||||
from hermes_state import SessionDB
|
||||
|
||||
db = SessionDB()
|
||||
db = SessionDB(read_only=True)
|
||||
except Exception:
|
||||
pass
|
||||
try:
|
||||
@@ -1336,11 +1338,13 @@ def _create_titled_session(title: str) -> Optional[str]:
|
||||
"""
|
||||
db = None
|
||||
try:
|
||||
from hermes_state import SessionDB
|
||||
from hermes_state_ids import new_session_id as mint_session_id
|
||||
from hermes_state_registry import acquire
|
||||
|
||||
new_session_id = mint_session_id()
|
||||
db = SessionDB()
|
||||
# The CLI acquires the registry handle for this same path moments later; share it
|
||||
# instead of minting a second writer for one INSERT (close() releases the refcount).
|
||||
db = acquire()
|
||||
db.create_session(new_session_id, source="cli")
|
||||
db.set_session_title(new_session_id, title)
|
||||
return new_session_id
|
||||
|
||||
@@ -41,7 +41,7 @@ def _print_tui_exit_summary(session_id: Optional[str], active_session_file: Opti
|
||||
db = None
|
||||
try:
|
||||
from hermes_state import SessionDB
|
||||
db = SessionDB()
|
||||
db = SessionDB(read_only=True) # exit epilogue only reads
|
||||
session = db.get_session(target)
|
||||
if not session:
|
||||
return
|
||||
|
||||
@@ -267,11 +267,13 @@ def run_oneshot(
|
||||
|
||||
def _create_session_db_for_oneshot():
|
||||
"""Best-effort SessionDB — oneshot bypasses ``HermesCLI._init_agent()``, so it must wire the
|
||||
SQLite store itself or ``session_search`` is advertised but always unavailable."""
|
||||
SQLite store itself or ``session_search`` is advertised but always unavailable. The registry
|
||||
handle is the one in-process tools (delegation, goals) acquire during the run, so the process
|
||||
holds one writer; ``_close_agent``'s ``close()`` releases the refcount."""
|
||||
try:
|
||||
from hermes_state import SessionDB
|
||||
from hermes_state_registry import acquire
|
||||
|
||||
return SessionDB()
|
||||
return acquire()
|
||||
except Exception as exc:
|
||||
logging.debug("SQLite session store not available for oneshot mode: %s", exc)
|
||||
return None
|
||||
|
||||
@@ -278,7 +278,7 @@ def _render_sessions(ctx):
|
||||
# pre-migration installs.
|
||||
try:
|
||||
from hermes_state import SessionDB
|
||||
db = SessionDB()
|
||||
db = SessionDB(read_only=True) # status only reads; never a writer beside a running gateway
|
||||
try:
|
||||
gateway_rows = db.list_gateway_sessions(active_only=True) or []
|
||||
finally:
|
||||
|
||||
@@ -126,7 +126,7 @@ def resolve_breadcrumb_session() -> Optional[str]:
|
||||
try:
|
||||
from hermes_state import SessionDB
|
||||
|
||||
db = SessionDB()
|
||||
db = SessionDB(read_only=True) # existence + lineage lookup only; no writer connection
|
||||
except Exception:
|
||||
return None
|
||||
try:
|
||||
|
||||
@@ -160,7 +160,10 @@ class _PathReadBudget:
|
||||
def register(self, db: "SessionDB") -> None:
|
||||
with self._lock:
|
||||
self._members.add(db)
|
||||
handles = len(self._members)
|
||||
# Only writable handles carry the cost the warning names (writer connection, write
|
||||
# lock, close-time checkpoint). Read-only attaches (dashboard routers, status/lookup
|
||||
# one-shots) open per request by design and must not trip it.
|
||||
handles = sum(1 for member in self._members if not member.read_only)
|
||||
warn = (handles > _HANDLES_PER_PATH_WARN and not self._duplicate_handles_warned)
|
||||
if warn:
|
||||
self._duplicate_handles_warned = True
|
||||
|
||||
@@ -550,13 +550,17 @@ def scan_sessions(limit: Optional[int] = None, progress_callback: Optional[Any]
|
||||
"""
|
||||
try:
|
||||
from hermes_state import SessionDB
|
||||
|
||||
# The scan only reads. A writable open here was a second writer connection (schema
|
||||
# init, write lock, close-time checkpoint) beside the dashboard's own store on every
|
||||
# scan, and counted toward the "live SessionDB handles" precursor (#100896).
|
||||
db = SessionDB(read_only=True)
|
||||
except Exception as exc:
|
||||
return {"sessions": [], "aggregate": {}, "error": f"Could not import SessionDB: {exc}", "scan_meta": _scan_meta("failed", 0)}
|
||||
return {"sessions": [], "aggregate": {}, "error": f"Could not open SessionDB: {exc}", "scan_meta": _scan_meta("failed", 0)}
|
||||
|
||||
previous_sessions = load_checkpoint()["sessions"] # load_checkpoint guarantees a dict
|
||||
reused = rescanned = 0
|
||||
db_limit = -1 if (limit is None or limit <= 0) else int(limit)
|
||||
db = SessionDB()
|
||||
try:
|
||||
sessions_meta = db.list_sessions_rich(limit=db_limit, include_children=True, project_compression_tips=False)
|
||||
total_sessions = len(sessions_meta)
|
||||
|
||||
@@ -72,7 +72,9 @@ def _owns_result(owner: str, parent: str | None) -> bool:
|
||||
return True
|
||||
from hermes_state import SessionDB
|
||||
|
||||
db = SessionDB()
|
||||
# Pure lineage read on the hot path of every retained-result load; a writable open here
|
||||
# was one more writer handle per call inside the gateway (#100896).
|
||||
db = SessionDB(read_only=True)
|
||||
try:
|
||||
return db.get_compression_tip(parent) == owner
|
||||
finally:
|
||||
|
||||
Reference in New Issue
Block a user