From 37686f33e6fc494f905543d276d7b2744144cda5 Mon Sep 17 00:00:00 2001 From: LeSingh1 Date: Sun, 9 Aug 2026 13:56:00 -0700 Subject: [PATCH] fix(core): credit an entry that vanishes at unlink time during eviction `_enforce_size_cap` walks its snapshot and subtracts each evicted entry's size from `total` until it is back under the cap. It handles "the file is already gone" twice, inconsistently: try: stat_now = path.stat() except FileNotFoundError: total -= size # credited continue ... try: _unlink_with_sharing_retry(path) total -= size except FileNotFoundError: pass # NOT credited Both are the same condition -- another process removed the entry -- observed a few microseconds apart, and the bytes are off disk either way. Not crediting the second one leaves `total` above the cap, so the pass evicts a second, live entry that did not need to go, and then reseeds `self._tracked_size_bytes = total` with that same overcount, which makes the next write trip the cap early and over-evict again. This is a documented, expected race for this backend: the class is designed for multi-process use, and a concurrent `__delitem__` or another process's eviction pass hits exactly this window. The `PermissionError` branch is now an explicit `continue`: an exhausted Windows sharing-violation retry leaves the file on disk, so its bytes must stay in `total`. That was already the behaviour; making it explicit keeps the single `total -= size` at the end honest. --- .../core/utils/_program_cache/_file_stream.py | 12 ++++- cuda_core/docs/source/release/1.2.0-notes.rst | 8 +++ cuda_core/tests/test_program_cache.py | 49 +++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/cuda_core/cuda/core/utils/_program_cache/_file_stream.py b/cuda_core/cuda/core/utils/_program_cache/_file_stream.py index eb71abf5446..504e7fb3bae 100644 --- a/cuda_core/cuda/core/utils/_program_cache/_file_stream.py +++ b/cuda_core/cuda/core/utils/_program_cache/_file_stream.py @@ -756,12 +756,22 @@ def _enforce_size_cap(self) -> None: # problems -- surface them rather than silently exceed the cap. try: _unlink_with_sharing_retry(path) - total -= size except FileNotFoundError: + # Someone else unlinked it between the stat-guard above and + # here. The bytes are off disk either way, so credit them -- + # exactly as the stat-miss branch a few lines up already + # does. Leaving ``total`` unchanged keeps it above the cap, + # so this pass evicts a live entry that did not need to go, + # and then reseeds the tracker with the same overcount, which + # makes the next write over-evict again. pass except PermissionError as exc: if not _is_windows_sharing_violation(exc): raise + # Retry budget exhausted on a Windows sharing violation: the + # file is still on disk, so its bytes must stay in ``total``. + continue + total -= size # Reconcile: after the eviction pass, ``total`` reflects what we # believe the disk now holds. Re-seed the tracker so the next write # accumulates from a fresh baseline. diff --git a/cuda_core/docs/source/release/1.2.0-notes.rst b/cuda_core/docs/source/release/1.2.0-notes.rst index 120d2c2a253..90bb973fd58 100644 --- a/cuda_core/docs/source/release/1.2.0-notes.rst +++ b/cuda_core/docs/source/release/1.2.0-notes.rst @@ -73,6 +73,14 @@ Fixes and enhancements Windows, both ``ctypes.CFUNCTYPE`` and ``ctypes.WINFUNCTYPE`` are accepted. (`#2439 `__) +- :class:`~cuda.core.utils.FileStreamProgramCache` no longer over-evicts when + another process removes an entry during a size-cap pass. The eviction loop + credited an entry that had already vanished by the time it was re-stat'ed, + but not one that vanished a few microseconds later at the ``unlink`` -- so + the pass believed it was still over the cap, evicted a live entry that did + not need to go, and reseeded the size tracker with the same overcount, + making the next write over-evict as well. + Deprecation Notices ------------------- diff --git a/cuda_core/tests/test_program_cache.py b/cuda_core/tests/test_program_cache.py index a8d3fc85f7e..5364feed57c 100644 --- a/cuda_core/tests/test_program_cache.py +++ b/cuda_core/tests/test_program_cache.py @@ -1784,6 +1784,55 @@ def test_filestream_cache_size_cap_counts_tmp_files(tmp_path): assert cache.get(b"c") is not None +@pytest.mark.agent_authored(model="claude-opus-5") +def test_filestream_size_cap_credits_an_entry_that_vanished_at_unlink(tmp_path): + """An entry another process removed mid-pass must be credited to ``total``. + + ``_enforce_size_cap`` already credits an entry that has vanished by the + time it re-stats it. The unlink a few lines later is the same "already + gone" condition a few microseconds later, but its ``FileNotFoundError`` + branch left ``total`` unchanged -- so the pass believed it was still over + the cap and evicted a second, live entry that did not need to go, then + reseeded the tracker with that same overcount, making the *next* write + over-evict too. + """ + from cuda.core.utils import FileStreamProgramCache + from cuda.core.utils._program_cache import _file_stream + + cap = 100 + with FileStreamProgramCache(tmp_path / "fc", max_size_bytes=cap) as cache: + for i in range(3): + time.sleep(0.02) # distinct atimes so eviction order is deterministic + cache[f"k{i}".encode()] = b"X" * 30 + assert cache._tracked_size_bytes == 90 + + real_unlink = _file_stream._unlink_with_sharing_retry + raced = [] + + def racing_unlink(path): + # The first victim of this pass is unlinked by "another process" + # in the window between our stat-guard and our own unlink. + if not raced: + raced.append(path) + path.unlink() + raise FileNotFoundError(path) + real_unlink(path) + + _file_stream._unlink_with_sharing_retry = racing_unlink + try: + time.sleep(0.02) + cache[b"k3"] = b"X" * 30 # 120 > 100 -> eviction pass runs + finally: + _file_stream._unlink_with_sharing_retry = real_unlink + + assert raced, "the eviction pass never reached an unlink" + # 30 bytes had to go to get under the cap and 30 bytes did go, so the + # three remaining entries must all survive. + assert len(cache) == 3 + assert cache._compute_total_size() == 90 + assert cache._tracked_size_bytes == 90 + + def test_filestream_cache_handles_long_keys(tmp_path): """Arbitrary-length keys must not overflow per-component filename limits. The filename is a fixed-length 256-bit digest; key uniqueness