fix(memory): share one SQLite connection per holographic store database

[adambiggs]

What

MemoryStore instances in the holographic memory plugin that point at the same database file now share a single process-wide SQLite connection and a single re-entrant lock, with refcounted close() semantics. The shared connection runs in autocommit (isolation_level=None), and the provider's shutdown() now releases its reference deterministically instead of leaving the connection to GC.

Why

Each MemoryStore opened its own connection guarded by its own per-instance RLock. Several providers coexist in one process — the main agent plus every delegate_task subagent — so instances on the same memory_store.db raced as independent WAL writers. Combined with writes that were not rolled back on error, one connection could leave an open write transaction that pinned the write lock, making every other connection's writes fail with sqlite3.OperationalError: database is locked for the full 10s busy timeout. We hit this repeatedly in production once delegated subagents started recording memories concurrently with the main agent.

Details of the fix:

• One connection + one RLock per database path, held in a class-level registry, so all access is serialized and cross-connection contention is impossible.
• Refcounted: closing one instance never tears the connection out from under a live sibling; the last close() releases it. close() is idempotent.
• Autocommit means a write that raises mid-method (e.g. during entity linking or bank rebuild) can never leave a dangling transaction holding the write lock. The existing explicit commit() calls become harmless no-ops, so the diff stays minimal.
• Schema init/WAL probe runs once per shared connection instead of once per instance.
• HolographicMemoryProvider.shutdown() now calls the refcount-guarded close() instead of just dropping the reference. Previously the connection (and its write lock) stayed alive until non-deterministic GC finalization — on a long-running gateway this prolongs the exact contention this PR removes, and it is the failure mode reported in the comments below (24/7 gateway + cron subprocesses). Same gap fix(memory): close SQLite in holographic shutdown + header validation (#44037) #44066 targeted.

How to test

scripts/run_tests.sh tests/plugins/memory/test_holographic_store.py

New tests cover: connection sharing per path, cross-instance write visibility, refcounted close (sibling survives, last close releases, idempotent double-close), context-manager release, reopen-after-close, 8 threads × 15 facts concurrent multi-instance writes with zero database is locked errors, write-lock release after a mid-write failure (in_transaction stays false), and provider shutdown() releasing the shared connection while a sibling provider stays live (both shutdown tests fail without the shutdown() wiring).

Full tests/plugins suite: 1182 passed, 0 failed.

To reproduce the original failure manually: run two Hermes agents (main + a delegate_task subagent) with the holographic memory provider on the same HERMES_HOME and have both store facts in a tight loop — pre-fix this intermittently raises database is locked.

Platforms

Tested on Linux (x86_64, Python 3.11). Pure-stdlib sqlite3/threading change, no platform-specific I/O.

🤖 Generated with Claude Code

[liuhao1024]

Verification review — reviewed the diff for correctness.

What I checked:

1. Shared connection registry — class-level _shared dict + _shared_guard mutex correctly serializes connection acquisition in __init__. Each db_path gets exactly one sqlite3.Connection and one RLock, refcounted across instances.
2. Autocommit mode (isolation_level=None) — every statement is its own transaction, preventing a failed write from pinning the SQLite write lock. Confirmed the test test_failed_write_does_not_pin_write_lock verifies in_transaction is False after a mid-method exception.
3. Refcount close() semantics — last close() tears down the connection; earlier closes just decrement. Double-close is idempotent (guarded by entry is None check). Tested in test_close_is_idempotent and test_closing_one_instance_keeps_sibling_alive.
4. Schema init guard — _init_db() runs exactly once per shared connection via the entry["ready"] flag, checked under the instance lock.
5. Thread safety of _shared_guard — protects all reads/writes to _shared in both __init__ and close(). No TOCTOU window (entry lookup and refcount bump happen inside the same with block).

The fix correctly eliminates the cross-connection WAL writer contention that caused intermittent "database is locked" failures when multiple MemoryStore instances (main agent + delegate_task subagents) targeted the same database file. The autocommit isolation level is the key insight — it prevents dangling transactions from blocking other connections.

Clean PR. No issues found.

[aurorabotticus-svg]

Been running Hermes for weeks with holographic memory enabled. Hit the "database is locked" wall hard today — traced it to cron jobs opening separate SQLite connections while the gateway holds the write lock. Found this PR. Read the description. 1182 tests pass. Reviewer approved it. Autocommit isolation level is the right fix. And yet it has been sitting here since June 10.

Patched it locally because I cannot wait for upstream. My gateway runs 24/7. This is not a nice-to-have — it is a data integrity issue. The old shutdown() never closes the connection, just drops the reference and prays to GC. On a long-running process that is not a bug, it is a time bomb.

Please merge this. Or at minimum merge #44066 (the shutdown fix) which is even simpler. Users should not have to reverse-engineer their own PRs from GitHub to keep their agent working.

[aurorabotticus-svg]

Update: this also explains a 32M token burn on a long-running gateway with barely any active chatting.

Every failed fact_store write returns "database is locked" to the model. The model sees the error, retries, tries different approaches, reasons about what went wrong — all of that generates tokens. Meanwhile on_memory_write mirrors every memory tool add into SQLite too, so even MEMORY.md writes hit the same lock. And prefetch runs search_facts before every turn, which tries to increment retrieval_count — another write that fails silently.

So the model is burning tokens on: failed writes, error recovery, compensatory reasoning with less context, and retry loops. All because shutdown() never closed the connection and cron jobs kept opening new ones. 32 million tokens of a model arguing with a locked database.

This is not cosmetic. This hits users in the wallet.

[adambiggs]

@aurorabotticus-svg Thanks for the detailed report — the 24/7-gateway + cron trace and the token-burn analysis were genuinely useful.

You were right about the shutdown gap: this PR added a refcount-guarded close() to MemoryStore, but HolographicMemoryProvider.shutdown() still just dropped the reference, so the teardown path never ran and the connection (plus its write lock) lived on until GC got around to it. On a long-running process that undermines the fix.

Now addressed in this PR (head 8305dfb21):

• shutdown() calls the idempotent, refcount-guarded close(), so the last provider releases the shared connection deterministically while sibling providers stay live — same gap fix(memory): close SQLite in holographic shutdown + header validation (#44037) #44066(a) targeted.
• Two regression tests cover the shutdown path (TestProviderShutdown); both fail without the wiring.

The header-validation half of #44066 is a separate hardening concern and intentionally out of scope here.

With the sharing + autocommit + deterministic release combined, the "database is locked" loop you traced (failed fact_store/on_memory_write/prefetch writes → model retry/reasoning burn) should be fully closed off. Hope this lands soon so you can drop the local patch.

[alt-glitch]

This was generated by AI during triage.

Related PR cluster for the holographic-memory "database is locked" symptom class (same goal, different mechanisms — NOT duplicates; a maintainer should pick the canonical fix):

• fix: retry holographic memory sqlite locks #40167 — busy_timeout + retry-on-lock.
• fix(memory): share one SQLite connection per holographic store database #43819 (this) — process-wide shared connection + re-entrant lock + autocommit (isolation_level=None) + refcounted close(), with HolographicMemoryProvider.shutdown() now releasing the shared connection deterministically.
• fix(memory): prevent holographic duplicate writes from locking db #55521 — autocommit-only for the duplicate-write path (narrower subset of this PR's mechanism).
• [BUG] Holographic memory provider permanent write lock on memory_store.db #55503 — the underlying bug report.
• fix(memory): close SQLite in holographic shutdown + header validation (#44037) #44066 (closed) — the connection-close half; this PR folds that gap in (header-validation intentionally left out of scope).

This PR looks the most comprehensive of the cluster (connection sharing + autocommit + deterministic release, third-party diff verification, regression tests covering the failed-write and teardown paths). A production report on this thread traced a persistent write-lock on a 24/7 gateway to ~32M tokens of retry/error-recovery burn and patched it locally.

[teknium1]

Merged via PR #61726 — your commit was cherry-picked onto current main with your authorship preserved in git log (rebase merge, commit a801046). Thanks @adambiggs — this was the most comprehensive fix in the locking cluster (#40167/#55521/#55503): the refcounted shared-connection registry plus autocommit removes the in-process database is locked contention and the dangling-transaction failure mode, and your test coverage (refcounting, 8-thread writers, failed-write lock release) made the salvage straightforward. We added one follow-up on top: the registry key is now Path.resolve()d so symlinked paths to the same DB share the connection.