hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 32 Packages Projects Releases Wiki Activity

fix(sessions): a failed retired-generation capture still pins the 3.11 handle and logs at ERROR

Browse Source 
Every production close of a SessionDB goes through hermes_state_registry.release_or_close,
whose teardown swallows any exception from close() at DEBUG. With the capture in place that
meant a RetiredGenerationCaptureError (disk full, permissions) left the handle open silently
and, on Python 3.11, skipped the retention pin: the raise happened before the pin, so the
interpreter's exit still ran sqlite3_close's implicit checkpoint and wrote the stale frames
over the newer generation (probe: 302 rows -> 4 after exit, worse than main).

- close() now decides retention first and takes the pin BEFORE attempting the capture on
  runtimes without setconfig; the capture failure is logged at ERROR and re-raised, so a
  later close() retries it while the pin already protects the newer generation.
- The registry logs RetiredGenerationCaptureError at ERROR instead of DEBUG (other teardown
  errors stay quiet). Same treatment on the release_or_close fallback path.
- The lost-generation settlement moves out of close() into _settle_lost_generation_locked();
  the sticky _close_checkpoint_disabled attribute and the dead "setconfig exists but failed"
  late-bind block are gone (_disable_close_time_checkpoint returns the call's outcome).

Regression test drives release_or_close with a failing capture: no raise, error text in the
log, pin taken exactly once (3.11), retry closes cleanly. Red on the salvaged head on both
3.11 and 3.12, green here.
This commit is contained in:
kshitijk4poor
2026-09-10 01:21:02 +05:30
committed by kshitij
parent d93f72c460
commit d87bf0d07e
3 changed files with 97 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

88
hermes_state.py
View File

@@ -462,8 +462,8 @@ class SessionDB(
# Durable capture of a lost WAL generation (see _capture_retired_generation): once per handle.
self._retired_generation_capture: Optional[Path] = None
self._retired_capture_lock = threading.Lock()
self._close_checkpoint_disabled = False # sticky once setconfig(NO_CKPT_ON_CLOSE) succeeded
self._retire_connection: Optional[Callable[[Any], None]] = None
self._connection_pinned = False # one unmatched C reference taken at most once per handle
self._db_corrupt, self._db_corrupt_reason = False, "" # sticky quarantine (StateDbCorruptError)
self._fts_usermerge_floor_applied = False # one-shot usermerge-floor write guard
self._fts_enabled = self._fts_stale = self._trigram_available = False
@@ -1108,16 +1108,56 @@ class SessionDB(
conn = self._conn
setconfig = getattr(conn, "setconfig", None)
if flag is None or setconfig is None:
return self._close_checkpoint_disabled
return False
try:
setconfig(flag, True)
self._close_checkpoint_disabled = True
except Exception:
logger.debug(
"Could not disable SQLite's close-time checkpoint on the quarantined handle for %s",
self.db_path, exc_info=True,
)
return self._close_checkpoint_disabled
return False
return True
def _pin_connection(self) -> None:
"""Retain the exact quarantined connection past GC and interpreter teardown (once per handle)."""
if not self._connection_pinned:
self._retire_connection(self._conn)
self._connection_pinned = True
def _settle_lost_generation_locked(self) -> bool:
"""Capture the retired generation; return whether the handle must be retired unclosed.
Where SQLite's close-time checkpoint cannot be switched off (no setconfig, Python < 3.12),
sqlite3_close would write the retired frames over the newer generation, so the exact
connection is retired unclosed instead. A failed capture leaves the handle open for a retry
-- but the pin is taken FIRST on such a runtime: every production caller reaches close()
through hermes_state_registry.release_or_close, which swallows the error, so an interpreter
exit before the retry must not be able to checkpoint the stale frames either."""
self._db_wal_generation_lost = True
retire_without_close = not self._disable_close_time_checkpoint() and self._retire_connection is not None
try:
artifact = self._capture_retired_generation("close")
except RetiredGenerationCaptureError as exc:
if retire_without_close:
self._pin_connection()
logger.error(
"Could not capture the retired WAL generation of %s at close: %s. The handle stays open "
"and close() retries the capture; those frames are NOT yet preserved.", self.db_path, exc,
)
raise
logger.warning(
"Skipping the close-time WAL checkpoint for %s: this handle's WAL/SHM generation "
"was deleted or replaced; the retired generation is captured at %s. Stop the other "
"writers before reopening and inspect the capture before deciding its disposition.",
self.db_path, artifact,
)
if retire_without_close:
logger.warning(
"Retaining the quarantined connection for %s unclosed: this runtime cannot "
"switch off SQLite's close-time checkpoint.", self.db_path,
)
return retire_without_close
def _raise_if_db_corrupt(self) -> None:
if self._db_corrupt:
@@ -1220,38 +1260,9 @@ class SessionDB(
self._db_wal_generation_lost
or (bool(self._db_sidecar_identity) and self._wal_generation_was_lost())
)
retire_without_close = False
if generation_lost:
# Loss is settled here, not at exit: the unlinked WAL inode dies with this process's
# last descriptor. Capture the exact retired generation before the handle goes and
# refuse to settle without it (the capture raises; the handle stays open).
self._db_wal_generation_lost = True
checkpoint_disabled = self._disable_close_time_checkpoint()
artifact = self._capture_retired_generation("close")
logger.warning(
"Skipping the close-time WAL checkpoint for %s: this handle's WAL/SHM generation "
"was deleted or replaced; the retired generation is captured at %s. Stop the other "
"writers before reopening and inspect the capture before deciding its disposition.",
self.db_path, artifact,
)
# Where SQLite's own close-time checkpoint cannot be switched off (no setconfig,
# Python < 3.12), sqlite3_close would still write the retired frames over the newer
# generation: retire the exact connection unclosed instead.
retire_without_close = not checkpoint_disabled
if retire_without_close and self._retire_connection is None:
try: # setconfig exists but failed at runtime: bind the capability late
self._retire_connection = _prepare_connection_retirement()
except RuntimeError as exc:
retire_without_close = False
logger.error(
"Cannot retain the quarantined connection for %s (%s); closing it may let "
"SQLite checkpoint retired frames over the newer generation.", self.db_path, exc,
)
if retire_without_close:
logger.warning(
"Retaining the quarantined connection for %s unclosed: this runtime cannot "
"switch off SQLite's close-time checkpoint.", self.db_path,
)
# Loss is settled here, not at exit: the unlinked WAL inode dies with this process's
# last descriptor (the capture raises and the handle stays open when it fails).
retire_without_close = generation_lost and self._settle_lost_generation_locked()
quarantine_reason = None if generation_lost else self._quarantine_reason()
if quarantine_reason is not None:
logger.warning(
@@ -1271,10 +1282,7 @@ class SessionDB(
except Exception as exc:
logger.debug("WAL checkpoint (PASSIVE) at close failed: %s", exc)
if retire_without_close:
# One unmatched C reference pins this exact connection past GC and
# module cleanup. Acquire it before detaching; repeated close() is
# then inert. Do not inspect or mutate other owners' descriptors.
self._retire_connection(self._conn)
self._pin_connection()
self._conn = None
else:
conn, self._conn = self._conn, None

19
hermes_state_registry.py
View File

@@ -108,10 +108,20 @@ def _teardown(db: "SessionDB") -> None:
"""Close a shared instance, clearing its registry-owned flag first."""
with contextlib.suppress(Exception):
db._shared_registry_owned = False
_close_quietly(db, "Error closing shared SessionDB")
def _close_quietly(db: "SessionDB", debug_message: str) -> None:
"""close() that never propagates. A lost WAL generation whose capture failed is data at risk,
not teardown noise: the handle stays open and the operator has to act, so that one surfaces."""
try:
db.close()
except Exception:
logger.debug("Error closing shared SessionDB", exc_info=True)
except Exception as exc:
from hermes_state_dbfile import RetiredGenerationCaptureError
if isinstance(exc, RetiredGenerationCaptureError):
logger.error("SessionDB for %s did not settle at close: %s", _db_path_of(db), exc)
else:
logger.debug(debug_message, exc_info=True)
def _path_lifecycle_lock_locked(path: Path) -> threading.Lock:
@@ -378,10 +388,7 @@ def release_or_close(db: "SessionDB") -> None:
"""Release a shared instance, or close it when it is not registry-managed. Drop-in for a
plain ``db.close()``: read-only opens, CLI one-shots and test fakes fall back."""
if not release(db):
try:
db.close()
except Exception:
logger.debug("release_or_close fallback close failed", exc_info=True)
_close_quietly(db, "release_or_close fallback close failed")
# ---- BEGIN PLUGIN-COMPAT (revert-scheduled; see COMPAT_MANIFEST.md) ----

36
tests/hermes_state/test_retired_wal_generation_capture.py
View File

@@ -207,6 +207,42 @@ def test_close_refuses_to_settle_without_a_capture(tmp_path, force_wal, monkeypa
recovered.close()
@not_windows
def test_failed_capture_still_pins_the_handle_and_surfaces_through_the_registry(tmp_path, force_wal, monkeypatch, caplog):
"""Production closes go through hermes_state_registry.release_or_close, which swallows close()
errors. A failed capture must still (a) log above DEBUG and (b) on runtimes without setconfig take
the retention pin, so an interpreter exit before the retry cannot checkpoint the stale frames."""
from hermes_state_registry import release_or_close
path = tmp_path / "state.db"
db = _make_db(path, "gw-0", "seed")
_require_wal(db)
_wal_only_sentinel(db, "gw-0")
_lose_sidecars(path, rename=False)
pins = []
if db._retire_connection is not None:
monkeypatch.setattr(db, "_retire_connection", pins.append)
def refuse(*args, **kwargs):
raise RetiredGenerationCaptureError("no space left on device")
monkeypatch.setattr(hermes_state, "capture_retired_wal_generation", refuse)
with caplog.at_level("ERROR"):
release_or_close(db) # must not raise
assert db._conn is not None
assert "no space left on device" in caplog.text
if db._retire_connection is not None: # no setconfig: the pin must already be taken
assert pins == [db._conn]
monkeypatch.undo()
monkeypatch.setattr(db, "_retire_connection", pins.append)
db.close() # retry succeeds and must not pin a second time
assert pins == [pins[0]]
else:
monkeypatch.undo()
db.close()
assert db._conn is None and db._retired_generation_capture is not None
@not_windows
def test_capture_selects_the_recorded_inode_not_the_pathname(tmp_path, force_wal):
"""A second deleted WAL under the same pathname belongs to another owner: it must be neither