mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
fix(state): enforce synchronous=FULL on macOS to prevent btree corruption
On Darwin, the default synchronous=NORMAL only calls fsync(), which Apple explicitly states does not guarantee data-on-platter or write-ordering. During a WAL checkpoint race with process termination (e.g., launchd shutdown), this can leave the main DB with half-written btree pages, resulting in btreeInitPage error 11 corruption. WAL mode's durability guarantee assumes the OS honors fsync barriers; macOS does not unless we explicitly set synchronous=FULL (which issues fsync() and F_FULLFSYNC via checkpoint_fullfsync=1). Previously, apply_wal_with_fallback() skipped setting synchronous=FULL when the DB was already in WAL mode, leaving connections at the unsafe synchronous=NORMAL default. This commit adds _enforce_macos_synchronous_full() to always enforce synchronous=FULL on macOS after any WAL activation. Fixes #63531
This commit is contained in:
parent
03fbf6edbb
commit
9aba95b053
2 changed files with 99 additions and 0 deletions
|
|
@ -356,6 +356,34 @@ def _apply_macos_checkpoint_barrier(conn: sqlite3.Connection) -> None:
|
|||
pass
|
||||
|
||||
|
||||
def _enforce_macos_synchronous_full(conn: sqlite3.Connection) -> None:
|
||||
"""Enforce ``PRAGMA synchronous=FULL`` on macOS to prevent btree corruption.
|
||||
|
||||
On Darwin, the default ``synchronous=NORMAL`` only calls ``fsync()``,
|
||||
which Apple's fsync(2) man page explicitly states does *not* guarantee
|
||||
data-on-platter or write-ordering. During a WAL checkpoint race with
|
||||
process termination (e.g., launchd shutdown), this can leave the main
|
||||
DB with half-written btree pages → ``btreeInitPage error 11``.
|
||||
|
||||
WAL mode's durability guarantee assumes the OS honors fsync barriers;
|
||||
macOS does not unless we explicitly set ``synchronous=FULL`` (which
|
||||
issues ``fsync()`` *and* ``F_FULLFSYNC`` via checkpoint_fullfsync=1).
|
||||
|
||||
This function is called after any successful WAL activation (either
|
||||
from ``apply_wal_with_fallback()`` setting a fresh WAL or when probing
|
||||
an existing WAL mode). It ensures macOS connections always use FULL
|
||||
synchronous mode, even if a prior connection set ``synchronous=NORMAL``.
|
||||
|
||||
Best-effort: never raises.
|
||||
"""
|
||||
if sys.platform != "darwin":
|
||||
return
|
||||
try:
|
||||
conn.execute("PRAGMA synchronous=FULL")
|
||||
except sqlite3.OperationalError:
|
||||
pass
|
||||
|
||||
|
||||
def apply_wal_with_fallback(
|
||||
conn: sqlite3.Connection,
|
||||
*,
|
||||
|
|
@ -387,6 +415,7 @@ def apply_wal_with_fallback(
|
|||
current_mode = conn.execute("PRAGMA journal_mode").fetchone()
|
||||
if current_mode and current_mode[0] == "wal":
|
||||
_apply_macos_checkpoint_barrier(conn)
|
||||
_enforce_macos_synchronous_full(conn)
|
||||
return "wal"
|
||||
except sqlite3.OperationalError:
|
||||
pass
|
||||
|
|
@ -394,6 +423,7 @@ def apply_wal_with_fallback(
|
|||
try:
|
||||
conn.execute("PRAGMA journal_mode=WAL")
|
||||
_apply_macos_checkpoint_barrier(conn)
|
||||
_enforce_macos_synchronous_full(conn)
|
||||
return "wal"
|
||||
except sqlite3.OperationalError as exc:
|
||||
msg = str(exc).lower()
|
||||
|
|
|
|||
|
|
@ -5180,6 +5180,75 @@ class TestApplyWalProbe:
|
|||
assert not any("checkpoint_fullfsync" in sql for sql in conn.executed), (
|
||||
"checkpoint_fullfsync must not be issued off macOS"
|
||||
)
|
||||
assert not any("synchronous=FULL" in sql for sql in conn.executed), (
|
||||
"synchronous=FULL must not be issued off macOS"
|
||||
)
|
||||
|
||||
def test_macos_synchronous_full_enforced_fresh(self, tmp_path, monkeypatch):
|
||||
"""On Darwin, apply_wal_with_fallback enforces synchronous=FULL (issue #63531)."""
|
||||
import sqlite3
|
||||
import hermes_state
|
||||
from hermes_state import apply_wal_with_fallback
|
||||
|
||||
class _TracingConn(sqlite3.Connection):
|
||||
def __init__(self, *a, **kw):
|
||||
super().__init__(*a, **kw)
|
||||
self.executed = []
|
||||
|
||||
def execute(self, sql, params=()):
|
||||
self.executed.append(sql)
|
||||
return super().execute(sql, params)
|
||||
|
||||
monkeypatch.setattr(hermes_state.sys, "platform", "darwin")
|
||||
|
||||
db_path = tmp_path / "macos_fresh_sync.db"
|
||||
conn = _TracingConn(str(db_path))
|
||||
try:
|
||||
result = apply_wal_with_fallback(conn)
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
assert result == "wal"
|
||||
assert any("synchronous=FULL" in sql for sql in conn.executed), (
|
||||
"synchronous=FULL must be enforced on macOS"
|
||||
)
|
||||
|
||||
def test_macos_synchronous_full_enforced_already_wal(self, tmp_path, monkeypatch):
|
||||
"""synchronous=FULL is enforced even when DB is already in WAL mode (issue #63531)."""
|
||||
import sqlite3
|
||||
import hermes_state
|
||||
from hermes_state import apply_wal_with_fallback
|
||||
|
||||
class _TracingConn(sqlite3.Connection):
|
||||
def __init__(self, *a, **kw):
|
||||
super().__init__(*a, **kw)
|
||||
self.executed = []
|
||||
|
||||
def execute(self, sql, params=()):
|
||||
self.executed.append(sql)
|
||||
return super().execute(sql, params)
|
||||
|
||||
# Prime the file into WAL mode first (simulating an existing WAL DB).
|
||||
db_path = tmp_path / "macos_wal_sync.db"
|
||||
with sqlite3.connect(str(db_path)) as seed:
|
||||
seed.execute("PRAGMA journal_mode=WAL")
|
||||
|
||||
monkeypatch.setattr(hermes_state.sys, "platform", "darwin")
|
||||
|
||||
conn = _TracingConn(str(db_path))
|
||||
try:
|
||||
result = apply_wal_with_fallback(conn)
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
assert result == "wal"
|
||||
# The early-return path for existing WAL must also enforce synchronous=FULL.
|
||||
assert any("synchronous=FULL" in sql for sql in conn.executed), (
|
||||
"synchronous=FULL must be enforced even on existing WAL DBs"
|
||||
)
|
||||
assert not any("journal_mode=WAL" in sql for sql in conn.executed), (
|
||||
"set-pragma must not run when already in WAL mode"
|
||||
)
|
||||
|
||||
def test_apply_wal_concurrent_connects_no_eio(self, tmp_path):
|
||||
"""20 threads calling connect() on the same DB must not see disk I/O error."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue