diff --git a/hermes_state.py b/hermes_state.py index c9399c7087..217e18435a 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -3787,6 +3787,14 @@ def _restore_journal_mode_after_repair( inside the repair path, not at open (the open-time flip #89393 warns about is a different door). + ``conn`` must be the exclusive repair guard connection when called from + the repair path (#101064): opening a fresh connection AFTER the guard + released let a writer still holding the unlinked old ``-wal`` inode + coexist with a brand-new ``state.db-wal`` this connection created — two + generations of one store. The transactional promotion already leaves the + destination in its pre-repair mode, so on that path this is mostly the + WAL-companion re-assertion; the reopen is the hazard, not the mode. + The restore runs through :func:`apply_wal_with_fallback` — the canonical journal-mode path — rather than issuing a switch pragma directly, so it inherits the vulnerable-SQLite WAL-reset gate (a rebuilt file IS a new diff --git a/tests/state/test_state_db_wal_unlink_race.py b/tests/state/test_state_db_wal_unlink_race.py index fea4af8656..5f8bf5271c 100644 --- a/tests/state/test_state_db_wal_unlink_race.py +++ b/tests/state/test_state_db_wal_unlink_race.py @@ -1,36 +1,84 @@ -"""Regression coverage for WAL restoration during state.db repair.""" +"""Regression coverage for WAL restoration during state.db repair (#101064). + +Journal-mode restoration used to open a NEW connection after the exclusive +repair guard had released the live database. In WAL mode a writer could still +hold the unlinked old WAL inode while that second connection created a fresh +``state.db-wal`` path — two generations of one store. The restore must run +through the guard connection, before the guard releases. +""" import sqlite3 import pytest import hermes_state +from hermes_state import repair_state_db_schema + + +def _make_db(path): + conn = sqlite3.connect(str(path), isolation_level=None) + conn.execute("CREATE TABLE sessions (name TEXT)") + conn.execute("INSERT INTO sessions VALUES ('seed')") + conn.close() -@pytest.mark.requires_wal def test_wal_restoration_reuses_exclusive_repair_connection(tmp_path, monkeypatch): - """WAL must be restored before the repair guard releases the live DB. - - Gated on ``requires_wal``: where the linked SQLite carries the WAL-reset - bug (or the filesystem cannot host WAL) ``apply_wal_with_fallback`` keeps - the store in DELETE by design, so the final ``== "wal"`` assertion would - fail for a reason unrelated to the connection-reuse contract. - """ + """Unit contract: given the guard connection, no reopen happens.""" db_path = tmp_path / "state.db" conn = sqlite3.connect(db_path, isolation_level=None) conn.execute("CREATE TABLE marker (value TEXT)") - conn.execute("PRAGMA journal_mode=DELETE") def fail_if_reopened(_path): pytest.fail("WAL restoration reopened state.db outside the repair guard") monkeypatch.setattr(hermes_state, "_connect_repair_durable", fail_if_reopened) - hermes_state._restore_journal_mode_after_repair( - db_path, - "delete", - conn=conn, - ) - - assert conn.execute("PRAGMA journal_mode").fetchone()[0].lower() == "wal" + hermes_state._restore_journal_mode_after_repair(db_path, None, conn=conn) + # The mode itself is whatever apply_wal_with_fallback resolves on this + # runtime (WAL, or DELETE on WAL-reset-vulnerable SQLite builds); the + # contract under test is the connection reuse, asserted above. + assert conn.execute("PRAGMA journal_mode").fetchone()[0].lower() in ("wal", "delete") conn.close() + + +def test_repair_never_reopens_after_the_guard_releases(tmp_path, monkeypatch): + """End to end through repair_state_db_schema: every connection the repair + opens is opened while the exclusive guard is still held, and none after.""" + db = tmp_path / "state.db" + _make_db(db) + monkeypatch.setattr(hermes_state, "_db_opens_cleanly", lambda path: "forced-unhealthy") + # The scratch-space pre-flight wants ~10GB headroom; irrelevant here. + monkeypatch.setattr(hermes_state, "_repair_scratch_space_error", lambda path: None) + + def fake_strategies(scratch_path, report): + report["repaired"] = True + report["strategy"] = "test_strategy" + return report + + monkeypatch.setattr(hermes_state, "_run_repair_strategies", fake_strategies) + + events: list[str] = [] + real_guard = hermes_state._exclusive_repair_db_guard + real_connect = hermes_state._connect_repair_durable + + from contextlib import contextmanager + + @contextmanager + def tracing_guard(path): + events.append("guard-enter") + with real_guard(path) as pair: + yield pair + events.append("guard-exit") + + def tracing_connect(path, *a, **kw): + events.append("connect") + return real_connect(path, *a, **kw) + + monkeypatch.setattr(hermes_state, "_exclusive_repair_db_guard", tracing_guard) + monkeypatch.setattr(hermes_state, "_connect_repair_durable", tracing_connect) + + report = repair_state_db_schema(db, backup=False) + assert report["repaired"] is True + assert "guard-exit" in events + after_release = events[events.index("guard-exit") + 1 :] + assert "connect" not in after_release, events