fix(state): a closing writer keeps its WAL generation until SQLite's own close (#121429)

SessionDB.close() lifts the OFD lock guard before sqlite3_close so a true
last close can still end the generation. When a stray in-process
open()/close() has already cancelled the handle's POSIX locks (the case
the guard exists for), lifting the OFD copy leaves the still-open
connection with no lock at all. A sibling closing in that gap takes
EXCLUSIVE and unlinks -wal/-shm under it, and every opener in the
meantime refuses with DeletedWalGenerationError.

release() now re-takes SQLite's own process-owned POSIX read lock on each
range before it drops the OFD copy. That is the lock SQLite still thinks
it holds. The re-take cannot be refused, because our OFD lock already
excludes writers. SQLite's close upgrades or drops the lock itself, so a
true last close still unlinks the generation.

Repro: the torture chamber's lock_cancellation episode on CI
(job 107586407677: "lock_cancellation-tui held state.db-shm (deleted)").
Deterministic with a sleep injected between release() and the connection
close: base red 4/4, fix green 4/4. The new lifecycle test runs a sibling
sqlite3 close at that seam: red on base (-shm unlinked), green on the fix,
and it checks that the last close still removes -wal.

hermes_state_lockguard.py

@@ -12,8 +12,9 @@ descriptors SQLite itself holds. OFD locks belong to the description, not the pr
 ``close()`` elsewhere cannot cancel them, they die with the connection's own descriptor (nothing
 extra to track or retire), and they conflict with a foreign EXCLUSIVE exactly like SQLite's own,
 so the sibling's close-time unlink is refused while a guarded handle is open. The guard is
-lifted before the handle's own close so a true last close still ends the generation normally.
-No-op on Windows and on runtimes without OFD locks.
+lifted before the handle's own close so a true last close still ends the generation normally;
+lifting it first re-takes SQLite's own POSIX locks on the same ranges, so the handle is never
+unlocked while its connection is still open. No-op on Windows and on runtimes without OFD locks.
 
 Ownership model: the guard is a property of the *descriptor*, and a descriptor number is
 reusable. Each ``hold()`` therefore locks every matching descriptor unconditionally (an OFD
@@ -49,7 +50,7 @@ _SHM_DMS_BYTE = 128
 if os.name == "nt":
     fcntl = None  # type: ignore[assignment]
     _F_OFD_SETLK: Optional[int] = None
-    _F_RDLCK = _F_UNLCK = _SEEK_SET = 0
+    _F_RDLCK = _F_UNLCK = _SEEK_SET = _F_SETLK = 0
 else:
     try:
         import fcntl
@@ -58,6 +59,7 @@ else:
         _F_OFD_SETLK = getattr(
             fcntl, "F_OFD_SETLK", {"linux": 37, "darwin": 90}.get(sys.platform.rstrip("0123456789")))
         _F_RDLCK, _F_UNLCK, _SEEK_SET = fcntl.F_RDLCK, fcntl.F_UNLCK, os.SEEK_SET
+        _F_SETLK = fcntl.F_SETLK
         # The constants alone do not make the guard usable: _ofd_lock() calls fcntl.fcntl(), so a
         # module that has the constants but no callable would pass supported() and then raise
         # from the first hold(). Probe the whole capability here, not just the symbols.
@@ -66,7 +68,7 @@ else:
     except (ImportError, AttributeError):
         fcntl = None  # type: ignore[assignment]
         _F_OFD_SETLK = None
-        _F_RDLCK = _F_UNLCK = _SEEK_SET = 0
+        _F_RDLCK = _F_UNLCK = _SEEK_SET = _F_SETLK = 0
 
 # struct flock differs per libc: glibc/musl put type+whence first, Darwin/BSD last.
 _FLOCK_FORMAT = "@qqihh" if sys.platform == "darwin" or "bsd" in sys.platform else "@hhqqi"
@@ -90,11 +92,12 @@ def _flock(lock_type: int, start: int, length: int) -> bytes:
     return struct.pack(_FLOCK_FORMAT, lock_type, _SEEK_SET, start, length, 0)
 
 
-def _ofd_lock(fd: int, lock_type: int, start: int, length: int) -> bool:
-    """Apply a non-blocking OFD lock; False when the range is held EXCLUSIVE elsewhere."""
+def _ofd_lock(fd: int, lock_type: int, start: int, length: int, *, cmd: Optional[int] = None) -> bool:
+    """Apply a non-blocking OFD lock (a process-owned POSIX one with ``cmd=_F_SETLK``); False when
+    the range is held EXCLUSIVE elsewhere."""
     assert fcntl is not None and _F_OFD_SETLK is not None
     try:
-        fcntl.fcntl(fd, _F_OFD_SETLK, _flock(lock_type, start, length))
+        fcntl.fcntl(fd, _F_OFD_SETLK if cmd is None else cmd, _flock(lock_type, start, length))
     except BlockingIOError:
         return False
     return True
@@ -165,7 +168,12 @@ def release(held: Held) -> None:
     """Drop this handle's claim. The last handle on an inode unlocks the range on every descriptor
     still referencing it. Call BEFORE the handle's own close so SQLite's close-time reset sees only
     real holders: a sibling process's intact locks still refuse the unlink, and a true last close
-    ends the generation, so a later ``state.db`` replace never pairs with a stale WAL."""
+    ends the generation, so a later ``state.db`` replace never pairs with a stale WAL.
+
+    The OFD copy is swapped back for the POSIX lock SQLite believes it still holds (a stray close
+    cancelled the real one): unlocked outright, the still-open connection would let a sibling's
+    close take EXCLUSIVE and unlink ``-wal``/``-shm`` under it before its own close runs. SQLite's
+    close upgrades or drops that process-owned lock itself, so a true last close is unaffected."""
     if not supported() or not held:
         return
     with _LOCK:
@@ -183,6 +191,7 @@ def release(held: Held) -> None:
         try:
             for fd, ident in _own_fds_for(set(to_unlock)):
                 start, length = to_unlock[ident]
+                _ofd_lock(fd, _F_RDLCK, start, length, cmd=_F_SETLK)  # never refused: our OFD lock excludes writers
                 _ofd_lock(fd, _F_UNLCK, start, length)
         except OSError:
             pass

tests/hermes_state/test_wal_lock_guard_lifecycle.py

@@ -71,3 +71,32 @@ def test_guard_never_outlives_the_handle_under_fd_reuse(tmp_path, monkeypatch):
     assert _foreign_exclusive_ok(str(path)), "a lock survived the last handle's close"
     assert not lg._HANDLES
     assert sqlite3.connect(path).execute("SELECT COUNT(*) FROM messages").fetchone()[0] == 2
+
+
+def test_closing_writer_stays_guarded_until_sqlite_closes(tmp_path, monkeypatch):
+    """close() lifts the OFD guard before SQLite's own close so a true last close still ends the
+    generation. A stray close had already cancelled this process's POSIX locks, so in that gap a
+    sibling's close could take EXCLUSIVE and unlink -wal/-shm under the still-open connection
+    (every concurrent opener then refuses with DeletedWalGenerationError)."""
+    pin_wal(monkeypatch)
+    path = tmp_path / "state.db"
+    db = make_db(path, "s", "seed")
+    require_wal(db)
+    for name in ("state.db", "state.db-shm"):  # the stray close the guard exists for
+        os.close(os.open(tmp_path / name, os.O_RDONLY))
+    shm = os.stat(f"{path}-shm").st_ino
+    in_gap: dict = {}
+    real_close = db._close_connection_quietly
+
+    def _sibling_closes_in_the_gap(conn):
+        subprocess.run([sys.executable, "-c",
+                        f"import sqlite3; c = sqlite3.connect({str(path)!r}); "
+                        "c.execute('select count(*) from messages').fetchone(); c.close()"], check=True)
+        in_gap["shm"] = os.stat(f"{path}-shm").st_ino if os.path.exists(f"{path}-shm") else None
+        real_close(conn)
+
+    monkeypatch.setattr(db, "_close_connection_quietly", _sibling_closes_in_the_gap)
+    db.close()
+    assert in_gap["shm"] == shm, "a sibling's close unlinked -shm under the closing writer"
+    assert not os.path.exists(f"{path}-wal"), "the true last close must still end the generation"
+    assert _foreign_exclusive_ok(str(path)) and not lg._HANDLES