Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions src/alignair/registry/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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:
Expand Down
27 changes: 27 additions & 0 deletions tests/alignair/registry/test_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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):
Expand Down
Loading