From 780da39caa0f65813094ad50d28b7b3feb6e07e1 Mon Sep 17 00:00:00 2001
From: thomas <thomaskon90@gmail.com>
Date: Sat, 18 Jul 2026 16:30:26 +0300
Subject: [PATCH] fix: close Windows lockfile contention race

---
 src/alignair/registry/cache.py       | 25 ++++++++++++++++++++-----
 tests/alignair/registry/test_lock.py | 27 +++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/src/alignair/registry/cache.py b/src/alignair/registry/cache.py
index 81b267b..a2e0b99 100644
--- a/src/alignair/registry/cache.py
+++ b/src/alignair/registry/cache.py
@@ -80,20 +80,27 @@ def _excl_lock(path: Path, *, timeout: float = 600.0, stale_after: float = 3600.
     holder's file older than ``stale_after``) so a crash can't deadlock the cache; time out otherwise."""
     import time
     fd = None
+    permission_miss = False
     deadline = time.monotonic() + timeout
     while True:
         try:
             fd = os.open(str(path), os.O_CREAT | os.O_EXCL | os.O_WRONLY)
             break
         except FileExistsError:
+            permission_miss = False
             pass                       # another holder owns the lockfile: contention
         except PermissionError:
             # Windows raises PermissionError (a sharing violation: WinError 32/5 -> errno 13) instead
             # of FileExistsError when another holder has the lockfile open. Treat that as contention,
-            # but only when the lockfile actually exists; a genuine permission failure (the path
-            # cannot be created at all) must propagate rather than spin until timeout.
+            # but only while the lockfile exists. The holder may release between os.open() failing
+            # and this existence check, so retry that disappearing-lock race once. A persistent
+            # permission failure with no lockfile still propagates promptly rather than timing out.
             if not os.path.exists(path):
-                raise
+                if permission_miss:
+                    raise
+                permission_miss = True
+                continue
+            permission_miss = False
         # contention: reclaim a stale lock (a dead holder's file), else wait until the deadline.
         try:
             age = time.time() - os.path.getmtime(path)
@@ -108,11 +115,19 @@ def _excl_lock(path: Path, *, timeout: float = 600.0, stale_after: float = 3600.
         if time.monotonic() > deadline:
             raise TimeoutError(f"timed out after {timeout}s acquiring cache lock {path}")
         time.sleep(poll)
+    # The lock is represented by the atomically-created file, not by an open handle. Closing the
+    # handle before the critical section avoids Windows sharing violations for other contenders;
+    # they can then observe the existing file through the normal FileExistsError path.
     try:
-        os.write(fd, str(os.getpid()).encode())
+        try:
+            os.write(fd, str(os.getpid()).encode())
+        finally:
+            os.close(fd)
+            fd = None
         yield
     finally:
-        os.close(fd)
+        if fd is not None:                         # acquisition succeeded but initialization failed
+            os.close(fd)
         try:
             os.unlink(path)
         except OSError:
diff --git a/tests/alignair/registry/test_lock.py b/tests/alignair/registry/test_lock.py
index 1ada232..aa1a07d 100644
--- a/tests/alignair/registry/test_lock.py
+++ b/tests/alignair/registry/test_lock.py
@@ -79,11 +79,14 @@ def test_excl_lock_propagates_genuine_permission_error(tmp_path, monkeypatch):
     """A real permission failure - the lockfile cannot be created and does NOT exist - must surface,
     not be mistaken for contention and spun until timeout."""
     lock = tmp_path / "noperm.lock"                                    # does not exist
+    attempts = 0
 
     real_open = os.open
 
     def fake_open(p, flags, *a, **k):
+        nonlocal attempts
         if str(p) == str(lock) and (flags & os.O_EXCL):
+            attempts += 1
             raise PermissionError(13, "access denied")                 # and the file is absent
         return real_open(p, flags, *a, **k)
 
@@ -92,6 +95,30 @@ def fake_open(p, flags, *a, **k):
     with pytest.raises(PermissionError):                              # not TimeoutError
         with _excl_lock(lock, timeout=0.5, poll=0.01):
             pass
+    assert attempts == 2                                               # one race retry, then propagate
+
+
+def test_excl_lock_retries_permission_error_when_holder_just_disappeared(tmp_path, monkeypatch):
+    """A Windows sharing violation can race with the holder deleting the lock before exists() runs.
+    Retry that transient absence instead of misclassifying it as a genuine permission failure."""
+    lock = tmp_path / "vanished.lock"
+    attempts = 0
+    real_open = os.open
+
+    def fake_open(p, flags, *a, **k):
+        nonlocal attempts
+        if str(p) == str(lock) and (flags & os.O_EXCL):
+            attempts += 1
+            if attempts == 1:
+                raise PermissionError(13, "sharing violation")         # holder released concurrently
+        return real_open(p, flags, *a, **k)
+
+    monkeypatch.setattr(os, "open", fake_open)
+
+    with _excl_lock(lock, timeout=0.5, poll=0.01):
+        assert lock.exists()
+    assert attempts == 2
+    assert not lock.exists()
 
 
 def test_cache_and_config_roots_honor_env_overrides(monkeypatch, tmp_path):