diff --git a/hermes_state_lockguard.py b/hermes_state_lockguard.py index de2532b934..74d456e6bb 100644 --- a/hermes_state_lockguard.py +++ b/hermes_state_lockguard.py @@ -40,17 +40,33 @@ _SHARED_FIRST = _PENDING_BYTE + 2 _SHARED_SIZE = 510 _SHM_DMS_BYTE = 128 -try: - import fcntl - # CPython exports F_OFD_SETLK only from 3.12. The kernel ABI values are stable: 37 on every - # Linux arch (asm-generic/fcntl.h), 90 on XNU (bsd/sys/fcntl.h, documented in fcntl(2)). - _F_OFD_SETLK: Optional[int] = 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 -except ImportError: # Windows +# Windows has no POSIX advisory locks, and the module promises to be a no-op there. Gate on the +# platform FIRST: some Windows installs have a third-party module importable as `fcntl` (stock +# CPython for Windows ships none), and letting the import decide would arm the guard on a +# lookalike. Off Windows a partial module is equally fatal at import time — every importer of +# hermes_state dies before supported() can say "no" (#118026) — so tolerate a missing attribute +# the same way as a missing module and fall back to the no-op. +if os.name == "nt": fcntl = None # type: ignore[assignment] - _F_OFD_SETLK = None + _F_OFD_SETLK: Optional[int] = None _F_RDLCK = _F_UNLCK = _SEEK_SET = 0 +else: + try: + import fcntl + # CPython exports F_OFD_SETLK only from 3.12. The kernel ABI values are stable: 37 on every + # Linux arch (asm-generic/fcntl.h), 90 on XNU (bsd/sys/fcntl.h, documented in fcntl(2)). + _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 + # 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. + if not callable(getattr(fcntl, "fcntl", None)): + raise ImportError("fcntl module has no fcntl() callable") + except (ImportError, AttributeError): + fcntl = None # type: ignore[assignment] + _F_OFD_SETLK = None + _F_RDLCK = _F_UNLCK = _SEEK_SET = 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" diff --git a/tests/hermes_state/test_lockguard_fcntl_tolerance.py b/tests/hermes_state/test_lockguard_fcntl_tolerance.py new file mode 100644 index 0000000000..7ead849c3e --- /dev/null +++ b/tests/hermes_state/test_lockguard_fcntl_tolerance.py @@ -0,0 +1,95 @@ +"""A partial or lookalike ``fcntl`` must disable the WAL lock guard, never kill the process. + +Regression for #118026: the module gated only on ``ImportError``, so a Windows install whose +venv has some other module importable as ``fcntl`` (stock CPython for Windows ships none) hit an +unguarded ``fcntl.F_RDLCK`` read at import time. ``hermes_state`` imports this module at module +level, so the ``AttributeError`` killed every importer — ``hermes serve`` (Desktop backend +"exited before port announcement (1)"), ``hermes doctor``, the gateway, cron — before +``supported()`` existed to report the guard as unavailable. +""" + +from __future__ import annotations + +import importlib +import os +import sys +import types + +import pytest + + +def _reimport_with_fcntl(monkeypatch, stub: types.ModuleType | None): + """Import the guard fresh with ``fcntl`` replaced (or absent), leaving sys.modules clean.""" + monkeypatch.delitem(sys.modules, "hermes_state_lockguard", raising=False) + + if stub is None: + monkeypatch.setitem(sys.modules, "fcntl", None) # import fcntl -> ImportError + else: + monkeypatch.setitem(sys.modules, "fcntl", stub) + + return importlib.import_module("hermes_state_lockguard") + + +def _windows_fcntl_lookalike() -> types.ModuleType: + """A stand-in exposing only flock/LOCK_*, as reported on the affected Windows installs.""" + stub = types.ModuleType("fcntl") + stub.flock = lambda *args, **kwargs: None + stub.LOCK_EX = 2 + stub.LOCK_SH = 1 + stub.LOCK_UN = 8 + return stub + + +def _constants_only_fcntl() -> types.ModuleType: + """The other partial shape: every lock constant present, no ``fcntl()`` callable. Passing the + constant checks alone would report ``supported()`` and then raise from the first hold().""" + stub = types.ModuleType("fcntl") + stub.F_OFD_SETLK = 37 + stub.F_RDLCK = 0 + stub.F_UNLCK = 2 + return stub + + +@pytest.mark.parametrize( + "stub", + [ + pytest.param(None, id="absent"), + pytest.param(_windows_fcntl_lookalike(), id="partial"), + pytest.param(_constants_only_fcntl(), id="constants-only"), + ], +) +def test_guard_degrades_to_a_no_op_instead_of_killing_every_importer(monkeypatch, tmp_path, stub): + guard = _reimport_with_fcntl(monkeypatch, stub) + + # An open descriptor on the file is what makes hold() reach fcntl.fcntl(); without one the + # constants-only shape would pass on a vacuous loop. + db = tmp_path / "state.db" + fd = os.open(db, os.O_RDWR | os.O_CREAT) + try: + # The contract the crash violated: the module imports, reports itself unavailable, and + # hold() hands back an empty guard so callers take the ordinary unguarded path. + assert guard.supported() is False + assert guard.hold(db) == {} + guard.release({}) + finally: + os.close(fd) + + +@pytest.mark.windows_only +def test_windows_never_arms_the_guard_even_with_an_importable_fcntl(monkeypatch): + # Stock CPython for Windows has no fcntl, so the lookalike is the only way this branch can + # see an importable module; the platform gate must decide before the import does. + guard = _reimport_with_fcntl(monkeypatch, _windows_fcntl_lookalike()) + + assert guard.fcntl is None + assert guard.supported() is False + assert guard.hold("state.db") == {} + + +def test_a_usable_fcntl_still_arms_the_guard(monkeypatch): + real_fcntl = pytest.importorskip("fcntl", reason="POSIX-only: nothing to arm on Windows") + + guard = _reimport_with_fcntl(monkeypatch, real_fcntl) + + # The tolerance above must not have disabled the guard everywhere. + assert guard.supported() is True