diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f6f41e..6e74247 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## Unreleased + +- Windows saves now use `msvcrt.locking` when `fcntl` is unavailable, so the + locked read-merge-write path is preserved instead of degrading to + atomic-write-only saves. The lock blocks indefinitely under contention, + matching `flock` semantics (`LK_LOCK` alone gives up after ~10s with + `OSError`). + ## 5.4.2 — 2026-07-02 Full-repository precision review (every file, line by line). Six defects diff --git a/mind.py b/mind.py index c0d34b0..729d146 100644 --- a/mind.py +++ b/mind.py @@ -494,11 +494,30 @@ def _save(self): except OSError as e: raise ValueError("refusing unsafe lock file %s: %s" % (lock_path, e)) with os.fdopen(lock_fd, "w") as lockf: + lock_backend = None try: import fcntl fcntl.flock(lockf.fileno(), fcntl.LOCK_EX) - except ImportError: # Windows: fall back to atomic write only - fcntl = None + lock_backend = ("fcntl", fcntl) + except ImportError: + try: + import msvcrt + except ImportError: # neither fcntl nor msvcrt: + lock_backend = None # degrade to atomic-write-only + else: + lockf.seek(0) + # LK_LOCK is not an indefinite blocking lock like flock: + # the CRT retries once per second, 10 times, then raises + # OSError — so a save contended for >10s would crash the + # very scenario the lock exists for. Keep waiting instead, + # exactly like the POSIX path. + while True: + try: + msvcrt.locking(lockf.fileno(), msvcrt.LK_LOCK, 1) + break + except OSError: + continue + lock_backend = ("msvcrt", msvcrt) try: if self.path.exists(): try: @@ -540,8 +559,13 @@ def _save(self): self._deleted.clear() self._pruned_edges.clear() finally: - if fcntl is not None: - fcntl.flock(lockf.fileno(), fcntl.LOCK_UN) + if lock_backend is not None: + name, module = lock_backend + if name == "fcntl": + module.flock(lockf.fileno(), module.LOCK_UN) + elif name == "msvcrt": + lockf.seek(0) + module.locking(lockf.fileno(), module.LK_UNLCK, 1) @staticmethod def _id(text): diff --git a/tests/test_mind.py b/tests/test_mind.py index 8042b8d..146f55a 100644 --- a/tests/test_mind.py +++ b/tests/test_mind.py @@ -8,6 +8,7 @@ import sys import tempfile import unittest +import builtins from datetime import datetime, timedelta from pathlib import Path @@ -761,6 +762,78 @@ def test_lock_symlink_does_not_truncate_target(self): h.remember("attacker-triggered write") self.assertEqual(victim.read_text("utf-8"), "precious") + def test_save_uses_msvcrt_lock_when_fcntl_is_unavailable(self): + """Windows must get a real file lock, not atomic-write-only saves.""" + h = self.hippo() + calls = [] + + class FakeMsvcrt: + LK_LOCK = 1 + LK_UNLCK = 2 + + @staticmethod + def locking(fd, mode, nbytes): + calls.append((mode, nbytes)) + + real_import = builtins.__import__ + + def fake_import(name, *args, **kwargs): + if name == "fcntl": + raise ImportError("fcntl is not available") + if name == "msvcrt": + return FakeMsvcrt + return real_import(name, *args, **kwargs) + + builtins.__import__ = fake_import + try: + h.remember("portable windows lock") + finally: + builtins.__import__ = real_import + + self.assertEqual(calls, [(FakeMsvcrt.LK_LOCK, 1), + (FakeMsvcrt.LK_UNLCK, 1)]) + + def test_msvcrt_lock_blocks_through_contention(self): + """LK_LOCK gives up with OSError after ~10s of contention; the save + must keep waiting like flock does, not crash — and must not lose + the write.""" + h = self.hippo() + calls = [] + + class ContendedMsvcrt: + LK_LOCK = 1 + LK_UNLCK = 2 + _denials = [2] # first two acquires collide + + @classmethod + def locking(cls, fd, mode, nbytes): + calls.append((mode, nbytes)) + if mode == cls.LK_LOCK and cls._denials[0] > 0: + cls._denials[0] -= 1 + raise OSError(36, "resource deadlock avoided") + + real_import = builtins.__import__ + + def fake_import(name, *args, **kwargs): + if name == "fcntl": + raise ImportError("fcntl is not available") + if name == "msvcrt": + return ContendedMsvcrt + return real_import(name, *args, **kwargs) + + builtins.__import__ = fake_import + try: + h.remember("survives lock contention") # must not raise + finally: + builtins.__import__ = real_import + + self.assertEqual(calls, [(ContendedMsvcrt.LK_LOCK, 1)] * 3 + + [(ContendedMsvcrt.LK_UNLCK, 1)]) + reloaded = Hippocampus(self.mind_dir / "graph.json") + self.assertTrue(any("contention" in n["text"] + for n in reloaded.nodes.values()), + "the contended save must still land on disk") + def test_archive_symlink_blocks_pruning(self): """'archived, not destroyed' is a guarantee: if the archive cannot be written, nothing is pruned."""