diff --git a/hermes_state.py b/hermes_state.py index 7c1cf4eaa6..a24142259b 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -2336,10 +2336,11 @@ def _cross_process_repair_lock(db_path: Path): """Serialize state.db schema surgery across processes. Yields True when this process holds the repair lock for *db_path*, False - when the bounded acquire timed out. Unlike the kanban init lock — whose - critical section is idempotent, so proceeding without the lock is merely - redundant work — proceeding here would be exactly the unsafe interleaving - we are trying to prevent, so a caller that gets False must NOT do surgery. + when the bounded acquire timed out or the lock file could not be opened at + all. Unlike the kanban init lock — whose critical section is idempotent, + so proceeding without the lock is merely redundant work — proceeding here + would be exactly the unsafe interleaving we are trying to prevent, so a + caller that gets False must NOT do surgery. ``flock`` is the right primitive for this: the kernel drops the lock when the holding process dies, so a crashed repairer cannot leave a stale lock @@ -2357,14 +2358,22 @@ def _cross_process_repair_lock(db_path: Path): lock_path.parent.mkdir(parents=True, exist_ok=True) handle = lock_path.open("a+b") except OSError as exc: - # Read-only dir, exhausted fds, exotic filesystem: fall back to the - # in-process behaviour that shipped before this lock existed rather - # than refusing to repair a DB we could otherwise heal. + # Fail closed, exactly as a timed-out acquire does. A lock file we + # cannot even open means the filesystem is out of space, inodes or + # descriptors — and a sibling that opened ITS handle before the disk + # filled is still inside writable_schema surgery or VACUUM. Yielding + # True here let two processes run schema surgery on the same live + # state.db concurrently, which is itself the corruption source this + # lock exists to remove (#100368: the disk-full trigger, then a fresh + # corruption on every boot with other writers alive). Callers already + # handle False by re-probing and reporting, and on a read-only + # directory no repair strategy could have written anyway. logger.warning( - "Could not open state.db repair lock %s (%s) — proceeding with " - "in-process serialisation only.", lock_path, exc, + "Could not open state.db repair lock %s (%s) — skipping schema " + "surgery rather than running it without cross-process authority.", + lock_path, exc, ) - yield True + yield False return acquired = False @@ -3614,16 +3623,19 @@ def repair_state_db_schema(db_path: Path, *, backup: bool = True) -> Dict[str, A result = report with _cross_process_repair_lock(db_path) as holding_lock: if not holding_lock: - # Another process is still inside its critical section. It may - # nonetheless have healed the file already (long VACUUM after a - # successful strategy), so re-probe before reporting failure. + # Another process is still inside its critical section, or the + # lock file itself could not be opened (full disk / no fds). It + # may nonetheless have healed the file already (long VACUUM after + # a successful strategy), so re-probe before reporting failure. if _db_opens_cleanly(db_path) is None: report["repaired"] = True report["strategy"] = "repaired_by_other_process" else: report["error"] = ( - "another process holds the state.db repair lock; skipped " - "schema surgery to avoid racing it" + "could not obtain the state.db repair lock (held by " + "another process, or the lock file was unopenable); " + "skipped schema surgery to avoid racing a concurrent " + "repairer" ) else: # The fast check above avoids taking the lock for a known-exhausted diff --git a/hermes_state_common.py b/hermes_state_common.py index c35b6135a1..ca081a603a 100644 --- a/hermes_state_common.py +++ b/hermes_state_common.py @@ -1132,10 +1132,11 @@ def fts_rebuild_admission(db_path): """Serialize full structural FTS rebuilds on *db_path* across processes. Yields True when this process holds the rebuild authority, False when the - bounded acquire timed out. A caller that gets False must NOT perform a - full rebuild — proceeding is exactly the concurrent-rebuild interleaving - this lock exists to prevent (fail closed). The deferred/stale breadcrumb - machinery already guarantees a skipped rebuild is retried later. + bounded acquire timed out or the lock file could not be opened at all. A + caller that gets False must NOT perform a full rebuild — proceeding is + exactly the concurrent-rebuild interleaving this lock exists to prevent + (fail closed). The deferred/stale breadcrumb machinery already guarantees + a skipped rebuild is retried later. ``db_path`` may be a str or Path; None (in-memory DB / tests without a file path) yields True — a private in-memory DB has no cross-process @@ -1148,13 +1149,22 @@ def fts_rebuild_admission(db_path): try: handle = open(lock_path, "a+b") except OSError as exc: - # Read-only dir, exhausted fds, exotic filesystem: fall back to the - # pre-lock behaviour rather than refusing a rebuild we could run. + # Fail closed, exactly as a timed-out acquire does. A lock file we + # cannot even open means the filesystem is out of space, inodes or + # descriptors — and a sibling process that opened ITS handle before + # the disk filled is still holding the authority and rebuilding. + # Yielding True here handed every process on a full disk a concurrent + # structural rebuild of the same live state.db with no cross-process + # authority at all: the disk-full trigger and the re-corruption on + # every multi-writer boot in #100368. Deferring costs nothing that + # was reachable anyway — the breadcrumb retries, and on a read-only + # directory the rebuild's own writes could not have committed either. logger.warning( - "Could not open FTS rebuild lock %s (%s) — proceeding with " - "in-process serialisation only.", lock_path, exc, + "Could not open FTS rebuild lock %s (%s) — deferring this rebuild " + "rather than running it without cross-process authority.", + lock_path, exc, ) - yield True + yield False return acquired = False