From 4544763d3044e9d7420ccc18f76c5d7425c63d1d Mon Sep 17 00:00:00 2001 From: "Aryan Singh K." <70511529+aryansk@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:33:37 +0530 Subject: [PATCH] fix: use real LRU eviction in DirCache The lru_cache-based eviction in DirCache never removed entries from _cache: lru_cache calls its wrapped function only on cache misses, so old keys were silently evicted from the recency index without popping _cached listings. Reads of evicted keys then deleted them lazily, and __iter__ (which filters through __getitem__) destroyed the whole cache. Track recency with an OrderedDict instead, evict the oldest entry on insert beyond max_paths, refresh order on access, and keep __delitem__ and __iter__ from corrupting state. Co-authored-by: CommandCodeBot --- fsspec/dircache.py | 19 ++++++++--- fsspec/tests/test_dircache.py | 64 +++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 fsspec/tests/test_dircache.py diff --git a/fsspec/dircache.py b/fsspec/dircache.py index eca19566b..fe9b294ca 100644 --- a/fsspec/dircache.py +++ b/fsspec/dircache.py @@ -1,6 +1,6 @@ import time +from collections import OrderedDict from collections.abc import MutableMapping -from functools import lru_cache class DirCache(MutableMapping): @@ -48,7 +48,7 @@ def __init__( self._cache = {} self._times = {} if max_paths: - self._q = lru_cache(max_paths + 1)(lambda key: self._cache.pop(key, None)) + self._q = OrderedDict() self.use_listings_cache = use_listings_cache self.listings_expiry_time = listings_expiry_time self.max_paths = max_paths @@ -58,11 +58,15 @@ def __getitem__(self, item): if self._times.get(item, 0) - time.time() < -self.listings_expiry_time: del self._cache[item] if self.max_paths: - self._q(item) + # Refresh the recency order; the item may already be evicted. + self._q.pop(item, None) + self._q[item] = None return self._cache[item] # maybe raises KeyError def clear(self): self._cache.clear() + if self.max_paths: + self._q.clear() def __len__(self): return len(self._cache) @@ -78,13 +82,20 @@ def __setitem__(self, key, value): if not self.use_listings_cache: return if self.max_paths: - self._q(key) + self._q.pop(key, None) + self._q[key] = None + while len(self._q) > self.max_paths: + oldest, _ = self._q.popitem(last=False) + self._cache.pop(oldest, None) + self._times.pop(oldest, None) self._cache[key] = value if self.listings_expiry_time is not None: self._times[key] = time.time() def __delitem__(self, key): del self._cache[key] + if self.max_paths: + self._q.pop(key, None) def __iter__(self): entries = list(self._cache) diff --git a/fsspec/tests/test_dircache.py b/fsspec/tests/test_dircache.py new file mode 100644 index 000000000..e3e58fdf9 --- /dev/null +++ b/fsspec/tests/test_dircache.py @@ -0,0 +1,64 @@ +import time + +from fsspec.dircache import DirCache + + +def test_max_paths_evicts_oldest_on_write(): + dc = DirCache(max_paths=2) + dc["a"] = 1 + dc["b"] = 2 + dc["c"] = 3 + dc["d"] = 4 + # Only the two most recent entries are retained. + assert len(dc) == 2 + assert dict(dc._cache) == {"c": 3, "d": 4} + + +def test_max_paths_iteration_does_not_destroy_cache(): + dc = DirCache(max_paths=2) + dc["a"] = 1 + dc["b"] = 2 + assert sorted(dc) == ["a", "b"] + # Iteration must not evict entries (regression: __iter__ used __getitem__, + # which popped entries out of the cache). + assert sorted(dc) == ["a", "b"] + assert len(dc) == 2 + + +def test_max_paths_access_refreshes_recency(): + dc = DirCache(max_paths=2) + dc["a"] = 1 + dc["b"] = 2 + assert dc["a"] == 1 # refresh 'a' + dc["c"] = 3 + # 'b' was the least recently used and should be evicted, not 'a'. + assert dict(dc._cache) == {"a": 1, "c": 3} + + +def test_no_max_paths_keeps_all(): + dc = DirCache() + for i in range(10): + dc[f"p{i}"] = i + assert len(dc) == 10 + assert sorted(dc) == [f"p{i}" for i in range(10)] + + +def test_deleted_path_removed_from_recency_tracking(): + dc = DirCache(max_paths=2) + dc["a"] = 1 + dc["b"] = 2 + del dc["a"] + dc["c"] = 3 + assert dict(dc._cache) == {"b": 2, "c": 3} + + +def test_expiry_still_applies(): + dc = DirCache(listings_expiry_time=0.1) + dc["a"] = 1 + time.sleep(0.2) + try: + dc["a"] + except KeyError: + pass + else: + raise AssertionError("expected expired entry to raise KeyError")