Release seqtree 1.3.2: reliable control build locking

.github/workflows/ci.yml

@@ -34,6 +34,9 @@ jobs:
       matrix:
         os: [ubuntu-latest, macos-latest]
         python: ["3.10", "3.12"]
+        include:
+          - os: windows-latest
+            python: "3.12"
     steps:
       - uses: actions/checkout@v4
       - uses: actions/setup-python@v5
@@ -46,7 +49,11 @@ jobs:
         continue-on-error: true
         run: uv pip install --system olga
       - name: Test
+        if: runner.os != 'Windows'
         run: pytest tests/python -q
+      - name: Windows control locking
+        if: runner.os == 'Windows'
+        run: pytest tests/python/test_control.py -q
 
   benchmarks:
     name: Benchmark tables + plots (oracle + perf)

docs/evalue.rst

@@ -68,9 +68,11 @@ precision bound below.
 
    That matters on *first* use in any multi-process fan-out — pytest-xdist, a Snakemake or Nextflow
    rule, a ``multiprocessing`` pool — all sharing one ``~/.cache``. Once the cache is warm it is
-   read-only and was never at risk. If ``filelock`` is installed (it arrives with
-   ``huggingface_hub``) the build is additionally serialized, so one worker builds and the rest
-   wait and load rather than all N rebuilding the same index; correctness does not depend on it.
+   read-only. Cold builds are serialized with an operating-system lock, including in a
+   base installation without ``filelock``. One worker builds and the rest wait and load.
+   Waiting is bounded to 600 seconds; an active owner causes a clear timeout. The kernel
+   releases ownership after an exception or process exit. Persistent ``.lock`` guard files
+   are intentional and must not be deleted while other processes may be waiting.
 
    A cache that is corrupt, truncated, or written by an older seqtree is silently rebuilt.
 

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build"
 
 [project]
 name = "seqtree"
-version = "1.3.1"
+version = "1.3.2"
 description = "Fast fuzzy search over biological sequences (C++ core, Python bindings)"
 readme = "README.md"
 requires-python = ">=3.10"

python/seqtree/control.py

@@ -122,24 +122,53 @@ def _prune_superseded(cache_dir, name, size, keep):
                 pass
 
 
-def _build_lock(cache):
-    """An inter-process lock around build-and-save, if ``filelock`` happens to be installed.
-
-    This is an *optimisation*, not the fix. ``Index.save`` writes to a temporary and renames it
-    into place, so a reader can never observe a half-written index whether or not this lock is
-    taken. What the lock buys is work: without it, a cold-cache fan-out of N workers has every
-    worker build the same 250k-clonotype index (seconds of CPU, 45 MB written) and N-1 of those
-    are thrown away. With it, one builds and the rest wait and load.
-
-    seqtree has no runtime dependencies, so ``filelock`` is used only if something else already
-    put it there -- ``huggingface_hub`` pulls it in, and that is installed whenever a control is
-    downloaded rather than read from the bundled asset.
+@contextlib.contextmanager
+def _build_lock(cache, timeout=600):
+    """Serialize cold builds without optional dependencies; process death releases the lock.
+
+    Keep the guard file: deleting it could let waiters lock different inodes.
     """
-    try:
-        from filelock import FileLock
-    except ImportError:
-        return contextlib.nullcontext()
-    return FileLock(cache + ".lock", timeout=600)
+    import errno
+    import time
+
+    with open(cache + ".lock", "a+b") as handle:
+        if os.name == "nt":
+            import msvcrt
+            if handle.seek(0, 2) == 0:
+                handle.write(b"\0")
+                handle.flush()
+
+            def acquire():
+                handle.seek(0)
+                msvcrt.locking(handle.fileno(), msvcrt.LK_NBLCK, 1)
+
+            def release():
+                handle.seek(0)
+                msvcrt.locking(handle.fileno(), msvcrt.LK_UNLCK, 1)
+        else:
+            import fcntl
+
+            def acquire():
+                fcntl.flock(handle.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)
+
+            def release():
+                fcntl.flock(handle.fileno(), fcntl.LOCK_UN)
+
+        deadline = time.monotonic() + timeout
+        while True:
+            try:
+                acquire()
+                break
+            except OSError as exc:
+                if exc.errno not in (errno.EACCES, errno.EAGAIN, errno.EDEADLK):
+                    raise
+                if time.monotonic() >= deadline:
+                    raise TimeoutError(f"Another control build owns {cache}.lock after {timeout:g}s") from exc
+                time.sleep(0.05)
+        try:
+            yield
+        finally:
+            release()
 
 
 def _load_cached(cache):

tests/python/test_cache_concurrency.py

@@ -169,19 +169,11 @@ def test_a_corrupt_cache_is_rebuilt_rather_than_raised(tmp_path):
     assert len(seqtree.Index.load(str(cache))) == n  # and the cache was rewritten in place
 
 
-def test_the_lock_is_optional_and_correctness_does_not_depend_on_it(monkeypatch):
-    """A bare ``pip install seqtree`` has no filelock -- it arrives only with huggingface_hub.
-
-    The lock saves redundant work; the atomic save is what makes concurrent use *correct*. If this
-    ever regresses into a hard import, a default install starts raising ImportError on first use.
-    """
+def test_the_lock_does_not_require_optional_filelock(tmp_path, monkeypatch):
+    """A base install serializes builds without importing an optional dependency."""
     import builtins
-
     from seqtree import control
 
-    assert control._build_lock("/tmp/x").__class__.__name__ in {"FileLock", "UnixFileLock",
-                                                                "WindowsFileLock", "nullcontext"}
-
     real_import = builtins.__import__
 
     def no_filelock(name, *args, **kwargs):
@@ -190,7 +182,10 @@ def no_filelock(name, *args, **kwargs):
         return real_import(name, *args, **kwargs)
 
     monkeypatch.setattr(builtins, "__import__", no_filelock)
-    lock = control._build_lock("/tmp/x")
-    assert lock.__class__.__name__ == "nullcontext"
-    with lock:  # must be a usable context manager, not a stub that explodes
+    cache = str(tmp_path / "control.idx")
+    with control._build_lock(cache):
+        with pytest.raises(TimeoutError):
+            with control._build_lock(cache, timeout=0):
+                pass
+    with control._build_lock(cache, timeout=0):
         pass

tests/python/test_control.py

@@ -178,3 +178,58 @@ def test_a_superseded_cache_is_not_served_and_is_cleaned_up(tmp_path):
     assert not legacy.exists(), "the superseded cache was left behind"
     assert ([p.name for p in tmp_path.iterdir() if p.suffix == ".sqtree"]
             == [control._cache_key("human_trb_aa", n, "aa", 0)])
+
+
+def test_control_build_lock_without_optional_filelock(tmp_path, monkeypatch):
+    import sys
+    import threading
+    import time
+    from seqtree.control import _build_lock
+
+    monkeypatch.setitem(sys.modules, "filelock", None)
+    ready = threading.Barrier(2)
+    state = {"active": 0, "peak": 0}
+    errors = []
+
+    def worker():
+        try:
+            ready.wait(timeout=5)
+            with _build_lock(str(tmp_path / "control.idx")):
+                state["active"] += 1
+                state["peak"] = max(state["peak"], state["active"])
+                time.sleep(0.1)
+                state["active"] -= 1
+        except BaseException as exc:
+            errors.append(exc)
+
+    workers = [threading.Thread(target=worker) for _ in range(2)]
+    for worker_thread in workers:
+        worker_thread.start()
+    for worker_thread in workers:
+        worker_thread.join(timeout=5)
+    assert not errors
+    assert not any(worker_thread.is_alive() for worker_thread in workers)
+    assert state["peak"] == 1
+    assert state["active"] == 0
+
+
+def test_control_lock_releases_after_failure_and_process_exit(tmp_path):
+    import subprocess
+    import sys
+    import pytest
+    from seqtree.control import _build_lock
+
+    cache = str(tmp_path / "control.idx")
+    with pytest.raises(RuntimeError, match="failed build"):
+        with _build_lock(cache):
+            raise RuntimeError("failed build")
+    with _build_lock(cache):
+        with pytest.raises(TimeoutError, match="Another control build"):
+            with _build_lock(cache, timeout=0.02):
+                pass
+    subprocess.run([sys.executable, "-c",
+                    "import os,sys; from seqtree.control import _build_lock; "
+                    "lock=_build_lock(sys.argv[1]); lock.__enter__(); os._exit(0)", cache],
+                   check=True, timeout=10)
+    with _build_lock(cache, timeout=1):
+        pass