fixup! refactor(core.utils): stat-guard atime touch; suppress platformdirs Windows Cache leaf

Author: cpcloud

cuda_core/cuda/core/utils/_program_cache.py

@@ -848,8 +848,11 @@ def _default_cache_dir() -> Path:
     Delegates to :mod:`platformdirs`, which encodes the per-platform
     rules canonically (and tracks corner cases like ``FOLDERID_*`` on
     Windows that an ad-hoc reimplementation would miss).
+    ``opinion=False`` suppresses the extra ``Cache`` component
+    platformdirs would otherwise insert on Windows, keeping the layout
+    identical across platforms (``<root>/cuda-python/program-cache``).
     """
-    return platformdirs.user_cache_path("cuda-python", appauthor=False) / "program-cache"
+    return platformdirs.user_cache_path("cuda-python", appauthor=False, opinion=False) / "program-cache"
 
 
 def _replace_with_sharing_retry(tmp_path: Path, target: Path) -> bool:
@@ -918,8 +921,9 @@ def _stat_and_read_with_sharing_retry(path: Path) -> tuple[os.stat_result, bytes
     raise FileNotFoundError(path) from last_exc
 
 
-def _touch_atime(path: Path, st: os.stat_result) -> None:
-    """Bump ``path``'s atime to "now", preserving its mtime.
+def _touch_atime(path: Path, st_before: os.stat_result) -> None:
+    """Bump ``path``'s atime to "now", preserving its mtime, iff the
+    file's stat still matches ``st_before``.
 
     Eviction sorts by ``st_atime`` so reads must reliably refresh atime
     regardless of OS or filesystem default behavior:
@@ -933,12 +937,31 @@ def _touch_atime(path: Path, st: os.stat_result) -> None:
     * ``noatime``-mounted filesystems disable updates entirely.
 
     Calling ``os.utime`` with explicit times bypasses all of the above
-    and writes atime directly. Best-effort: any ``OSError`` (read-only
-    mount, restrictive ACLs, ...) is swallowed -- size enforcement
-    still bounds the cache, but eviction degrades toward FIFO.
+    and writes atime directly. The stat-guard is critical: if another
+    process ``os.replace``-d a fresh entry into ``path`` between the
+    read and this touch, blindly applying ``st_before.st_mtime_ns``
+    would roll the new entry's mtime back to the old value and confuse
+    the eviction stat-guard (which checks ``(ino, size, mtime_ns)``)
+    into deleting a freshly-committed file. A mismatch means the racing
+    writer's reader will refresh atime when it next reads, so skipping
+    here is safe.
+
+    Best-effort: any ``OSError`` (read-only mount, restrictive ACLs,
+    ...) is swallowed -- size enforcement still bounds the cache, but
+    eviction degrades toward FIFO.
     """
+    try:
+        st_now = path.stat()
+    except OSError:
+        return
+    if (st_now.st_ino, st_now.st_size, st_now.st_mtime_ns) != (
+        st_before.st_ino,
+        st_before.st_size,
+        st_before.st_mtime_ns,
+    ):
+        return
     with contextlib.suppress(OSError):
-        os.utime(path, ns=(time.time_ns(), st.st_mtime_ns))
+        os.utime(path, ns=(time.time_ns(), st_before.st_mtime_ns))
 
 
 def _prune_if_stat_unchanged(path: Path, st_before: os.stat_result) -> None:

cuda_core/tests/test_program_cache.py

@@ -1169,6 +1169,40 @@ def test_filestream_cache_prune_only_if_stat_unchanged(tmp_path):
     assert not path.exists()
 
 
+def test_filestream_cache_touch_atime_only_if_stat_unchanged(tmp_path):
+    """The atime-touch is also stat-guarded so a racing rewriter's freshly
+    replaced file does NOT get its mtime rolled back to the previous
+    entry's value. Without the guard, the eviction stat-check (which keys
+    on (ino, size, mtime_ns)) would mistake the replacement for the old
+    entry and delete a just-committed file."""
+    from cuda.core.utils import FileStreamProgramCache
+    from cuda.core.utils._program_cache import _touch_atime
+
+    same_size_bytes = b"v" * 64
+    with FileStreamProgramCache(tmp_path / "fc") as cache:
+        cache[b"k"] = same_size_bytes
+        path = cache._path_for_key(b"k")
+        stale_stat = path.stat()
+        # Concurrent writer replaces with same-size payload (same st_size,
+        # different ino/mtime) -- this is the dangerous case: ino and
+        # mtime differ, only the stat-guard saves us.
+        time.sleep(0.02)
+        cache[b"k"] = same_size_bytes
+        new_mtime_ns = path.stat().st_mtime_ns
+
+    _touch_atime(path, stale_stat)
+    # The new file's mtime must be untouched.
+    assert path.stat().st_mtime_ns == new_mtime_ns
+
+    # With a stat that matches the current file, atime is updated and
+    # mtime is preserved.
+    fresh_stat = path.stat()
+    _touch_atime(path, fresh_stat)
+    after = path.stat()
+    assert after.st_mtime_ns == fresh_stat.st_mtime_ns
+    assert after.st_atime_ns >= fresh_stat.st_atime_ns
+
+
 def test_filestream_cache_returns_bytes_verbatim(tmp_path):
     """The cache stores raw binary and does no payload validation: whatever
     bytes were written are returned exactly. External tools (cuobjdump,
@@ -1242,13 +1276,22 @@ def test_filestream_cache_rejects_negative_size_cap(tmp_path):
 def test_default_cache_dir_lives_under_user_cache_root():
     """We delegate per-platform path resolution to ``platformdirs``, so we
     only verify the cuda-python-owned suffix; the OS-specific bits are
-    platformdirs' responsibility."""
+    platformdirs' responsibility.
+
+    ``opinion=False`` matters: with the default ``opinion=True`` on
+    Windows, platformdirs inserts an extra ``Cache`` component that
+    would diverge from the documented ``<root>/cuda-python/program-cache``
+    layout. Asserting against the same flag pins the layout invariant.
+    """
     import platformdirs
 
     from cuda.core.utils._program_cache import _default_cache_dir
 
-    expected = platformdirs.user_cache_path("cuda-python", appauthor=False) / "program-cache"
+    expected = platformdirs.user_cache_path("cuda-python", appauthor=False, opinion=False) / "program-cache"
     assert _default_cache_dir() == expected
+    # Path must end with cuda-python/program-cache regardless of platform;
+    # no extra "Cache" component sneaks in.
+    assert _default_cache_dir().parts[-2:] == ("cuda-python", "program-cache")
 
 
 def test_filestream_cache_uses_default_dir_when_path_omitted(tmp_path, monkeypatch):