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
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ Unreleased

- improve typing and refresh project to use pyproject and pallets workflows :pr:`420`
- replace deprecated ``datetime.utcnow()`` with ``datetime.now()`` :pr:`421`
- fix ``FileSystemCache`` permission errors on Network Attached Storage (NAS) when trying
to perform operations on files that are open in other processes :pr:`424`

Version 0.13.0
--------------
Expand Down
37 changes: 18 additions & 19 deletions src/cachelib/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,25 +317,24 @@ def has(self, key: str) -> bool:
def _run_safely(
self, fn: _t.Callable[..., _t.Any], *args: _t.Any, **kwargs: _t.Any
) -> _t.Any:
"""On Windows os.replace, os.chmod and open can yield
permission errors if executed by two different processes."""
if platform.system() == "Windows":
output = None
wait_step = 0.001
max_sleep_time = 10.0
total_sleep_time = 0.0

while total_sleep_time < max_sleep_time:
try:
output = fn(*args, **kwargs)
except PermissionError:
sleep(wait_step)
total_sleep_time += wait_step
wait_step *= 2
else:
break
else:
output = fn(*args, **kwargs)
"""On some filesystems (e.g. SMB/CIFS on Linux, NTFS on Windows)
os.replace, os.chmod and open can yield PermissionError when executed
by two different processes concurrently. Retries with exponential
backoff on all platforms."""
output = None
wait_step = 0.001
max_sleep_time = 10.0
total_sleep_time = 0.0

while total_sleep_time < max_sleep_time:
try:
output = fn(*args, **kwargs)
except PermissionError:
sleep(wait_step)
total_sleep_time += wait_step
wait_step *= 2
else:
break

return output

Expand Down
70 changes: 70 additions & 0 deletions tests/test_file_system_cache.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import hashlib
import os
from time import sleep
from unittest.mock import Mock
from unittest.mock import patch

import pytest
from clear import ClearTests
Expand Down Expand Up @@ -126,6 +128,74 @@ def test_prune_old_entries(self):
assert cache.has(f"{k}-t10")
assert not cache.has(f"{k}-t1")

def test_run_safely_succeeds_on_first_try(self):
"""_run_safely returns the function's result when no error is raised."""
cache = self.cache_factory()
result = cache._run_safely(lambda: 42)
assert result == 42

def test_run_safely_retries_on_permission_error(self):
"""
_run_safely retries when PermissionError is raised
(e.g. Windows NTFS race).
"""
cache = self.cache_factory()
flaky = Mock(
side_effect=[PermissionError("locked"), PermissionError("locked"), "ok"]
)

with patch("cachelib.file.sleep") as mock_sleep:
result = cache._run_safely(flaky)

assert result == "ok"
assert flaky.call_count == 3
assert mock_sleep.call_count == 2

def test_run_safely_exhausts_retries_and_returns_none(self):
"""
_run_safely returns None when PermissionError persists
beyond max_sleep_time.
"""
cache = self.cache_factory()

def always_fails():
raise PermissionError("locked")

with patch("cachelib.file.sleep"):
result = cache._run_safely(always_fails)

assert result is None

def test_run_safely_does_not_retry_other_errors(self):
"""_run_safely does not catch non-PermissionError exceptions."""
cache = self.cache_factory()

def raises_file_not_found():
raise FileNotFoundError("missing")

with pytest.raises(FileNotFoundError):
cache._run_safely(raises_file_not_found)

def test_run_safely_exponential_backoff(self):
"""_run_safely doubles the wait_step on each retry."""
cache = self.cache_factory()
flaky = Mock(
side_effect=[
PermissionError("locked"),
PermissionError("locked"),
PermissionError("locked"),
"ok",
]
)

with patch("cachelib.file.sleep") as mock_sleep:
cache._run_safely(flaky)

sleep_calls = [c.args[0] for c in mock_sleep.call_args_list]
# Each wait should double the previous
assert sleep_calls[1] == sleep_calls[0] * 2
assert sleep_calls[2] == sleep_calls[1] * 2

def test_mgmt_element_set_not_counted(self, tmpdir):
"""set with mgmt_element=True should not affect _file_count."""
cache = FileSystemCache(tmpdir)
Expand Down
Loading