Skip to content

Commit a9eb728

Browse files
Copilotshaypal5
andauthored
Fix PermissionError in _clear_all_cache_files on Windows (separate_files=True) (#348)
* Fix PermissionError in _clear_all_cache_files on Windows by adding retry loop Co-authored-by: shaypal5 <917954+shaypal5@users.noreply.github.com> * Add tests for _clear_all_cache_files retry logic to reach 100% patch coverage Co-authored-by: shaypal5 <917954+shaypal5@users.noreply.github.com> * Fix ruff-format pre-commit.ci failure in test_pickle_core.py Co-authored-by: shaypal5 <917954+shaypal5@users.noreply.github.com> * Add pragma no branch to inner for loop to reach 100% patch coverage Co-authored-by: shaypal5 <917954+shaypal5@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: shaypal5 <917954+shaypal5@users.noreply.github.com>
1 parent a599cf1 commit a9eb728

2 files changed

Lines changed: 88 additions & 1 deletion

File tree

src/cachier/cores/pickle.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,19 @@ def _clear_all_cache_files(self) -> None:
145145
path, name = os.path.split(self.cache_fpath)
146146
for subpath in os.listdir(path):
147147
if subpath.startswith(f"{name}_"):
148-
os.remove(os.path.join(path, subpath))
148+
fpath = os.path.join(path, subpath)
149+
# Retry loop to handle Windows mandatory file-locking (WinError 32):
150+
# portalocker holds an exclusive lock while a thread is computing,
151+
# so os.remove() may fail transiently until the lock is released.
152+
for attempt in range(3): # pragma: no branch
153+
try:
154+
os.remove(fpath)
155+
break
156+
except PermissionError:
157+
if attempt < 2:
158+
time.sleep(0.1 * (attempt + 1))
159+
else:
160+
raise
149161

150162
def _clear_being_calculated_all_cache_files(self) -> None:
151163
path, name = os.path.split(self.cache_fpath)

tests/pickle_tests/test_pickle_core.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,6 +1212,81 @@ def mock_func():
12121212
core.delete_stale_entries(timedelta(hours=1))
12131213

12141214

1215+
@pytest.mark.pickle
1216+
def test_clear_all_cache_files_retries_on_permission_error(tmp_path):
1217+
"""Test _clear_all_cache_files retries on PermissionError then succeeds."""
1218+
core = _PickleCore(
1219+
hash_func=None,
1220+
cache_dir=tmp_path,
1221+
pickle_reload=False,
1222+
wait_for_calc_timeout=10,
1223+
separate_files=True,
1224+
)
1225+
1226+
def mock_func():
1227+
pass
1228+
1229+
core.set_func(mock_func)
1230+
1231+
# Create a cache file that matches the name pattern
1232+
cache_fpath = core.cache_fpath
1233+
dummy_file = cache_fpath + "_dummykey"
1234+
with open(dummy_file, "wb") as f:
1235+
f.write(b"")
1236+
1237+
# os.remove fails twice then succeeds on the third call
1238+
real_remove = os.remove
1239+
call_count = 0
1240+
1241+
def flaky_remove(path):
1242+
nonlocal call_count
1243+
call_count += 1
1244+
if call_count < 3:
1245+
raise PermissionError("locked")
1246+
real_remove(path)
1247+
1248+
with (
1249+
patch("cachier.cores.pickle.os.remove", side_effect=flaky_remove),
1250+
patch("cachier.cores.pickle.time.sleep") as mock_sleep,
1251+
):
1252+
core._clear_all_cache_files()
1253+
assert mock_sleep.call_count == 2
1254+
mock_sleep.assert_any_call(0.1)
1255+
mock_sleep.assert_any_call(0.2)
1256+
1257+
assert not os.path.exists(dummy_file)
1258+
1259+
1260+
@pytest.mark.pickle
1261+
def test_clear_all_cache_files_raises_on_persistent_permission_error(tmp_path):
1262+
"""Test _clear_all_cache_files re-raises PermissionError after all retries."""
1263+
core = _PickleCore(
1264+
hash_func=None,
1265+
cache_dir=tmp_path,
1266+
pickle_reload=False,
1267+
wait_for_calc_timeout=10,
1268+
separate_files=True,
1269+
)
1270+
1271+
def mock_func():
1272+
pass
1273+
1274+
core.set_func(mock_func)
1275+
1276+
# Create a cache file that matches the name pattern
1277+
cache_fpath = core.cache_fpath
1278+
dummy_file = cache_fpath + "_dummykey"
1279+
with open(dummy_file, "wb") as f:
1280+
f.write(b"")
1281+
1282+
with (
1283+
patch("cachier.cores.pickle.os.remove", side_effect=PermissionError("locked")),
1284+
patch("cachier.cores.pickle.time.sleep"),
1285+
pytest.raises(PermissionError),
1286+
):
1287+
core._clear_all_cache_files()
1288+
1289+
12151290
# Redis core static method tests
12161291
@pytest.mark.parametrize(
12171292
("test_input", "expected"),

0 commit comments

Comments
 (0)