Skip to content

fix(core): validate program-cache max_size_bytes and reject an empty cache path - #2553

Open
LeSingh1 wants to merge 1 commit into
NVIDIA:mainfrom
LeSingh1:core-program-cache-arg-validation
Open

fix(core): validate program-cache max_size_bytes and reject an empty cache path#2553
LeSingh1 wants to merge 1 commit into
NVIDIA:mainfrom
LeSingh1:core-program-cache-arg-validation

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Problem

Two argument checks in the program caches accept values that make the cache silently useless.

max_size_bytes

Both backends (_in_memory.py, _file_stream.py) guard with a bare comparison:

if max_size_bytes is not None and max_size_bytes <= 0:
    raise ValueError("max_size_bytes must be positive or None (0 would evict every write)")

bool is an int subclass, so True sails through as a one-byte cap. The size enforcer then evicts every write the moment it lands:

>>> cache = InMemoryProgramCache(max_size_bytes=True)
>>> cache[b"k"] = b"ab"
>>> len(cache)
0

…while its twin False is rejected by that very line. The comment on the guard shows 0 was reasoned about carefully; bool was not.

Measured on main, both backends behave identically:

max_size_bytes result on main should be
100, None accepted accepted
0, -1 ValueError ValueError
False ValueError ValueError
True accepted → discards every write ValueError
1.5 accepted ValueError
"100" TypeError: '<=' not supported between instances of 'str' and 'int' ValueError

path

self._root = Path(path) if path is not None else _default_cache_dir()

Path("") is Path("."). So an empty path roots the cache in the current working directory, and __init__ immediately creates entries/ and tmp/ there — directories a later clear() will rmdir. Verified: FileStreamProgramCache("") from a scratch cwd leaves ['entries', 'tmp'] behind.

path=os.environ.get("MY_CACHE_DIR", "") is how a caller lands on it — and the asymmetry is in the same file: _default_cache_dir() deliberately treats an empty XDG_CACHE_HOME / LOCALAPPDATA as unset:

xdg = os.environ.get("XDG_CACHE_HOME")
root = Path(xdg) if xdg else Path.home() / ".cache"

Confirmed: XDG_CACHE_HOME="" correctly falls back to ~/.cache/cuda-python/program-cache. Empty means "unset" for the env var and "cwd" for the parameter.

Fix

  • max_size_bytes: require a positive int, using the same isinstance(x, bool) or not isinstance(x, int) shape already used by Host.__new__ and checkpoint._check_pid. The message keeps the word positive, which the existing tests match on, and now echoes the offending value.
  • path: reject the empty string with a message pointing at None for the default, rather than silently defaulting (per the repo's "no success-shaped fallbacks" guidance). An explicit "." still works, and path=None is untouched.

Tests

Added next to the existing cap tests in cuda_core/tests/test_program_cache.py:

  • test_{filestream,inmemory}_cache_rejects_non_int_size_cap over True, 1.5, "100";
  • test_filestream_cache_rejects_an_empty_path, which also asserts nothing was created in the cwd;
  • test_filestream_cache_still_accepts_an_explicit_dot, so the fix does not over-reach.

The existing [-1, 0] parametrization and the path=None default-directory tests are untouched.

Verification I could and could not do

  • _in_memory.py / _file_stream.py import only stdlib plus cuda.core._module.ObjectCode, so I loaded them by path with a three-line stub for that one symbol and ran every new case against both upstream/main and the fix:
--- WITH FIX ---            all 7 cases -> ValueError (tests pass)
--- upstream/main ---
  InMemory(max_size_bytes=True)   NOT REJECTED            -> test fails
  FileStream(max_size_bytes=True) NOT REJECTED            -> test fails
  InMemory(max_size_bytes=1.5)    NOT REJECTED            -> test fails
  FileStream(max_size_bytes=1.5)  NOT REJECTED            -> test fails
  InMemory(max_size_bytes='100')  TypeError               -> pytest.raises(ValueError) fails
  FileStream(max_size_bytes='100') TypeError              -> pytest.raises(ValueError) fails
  FS("")  NOT REJECTED, created ['entries', 'tmp']        -> test fails
  • ruff check / ruff format --check and python -m py_compile clean on all changed files.
  • Not run: pytest cuda_core/tests/test_program_cache.py itself — it imports cuda.core.utils, and cuda.core is not importable here (no CUDA driver, no built extension modules). The stub run above exercises the same constructor code paths the new tests do. Please treat CI as the first real run.

Two argument checks in the program caches accept values that make the cache
silently useless.

max_size_bytes
--------------
Both backends guard with a bare comparison:

    if max_size_bytes is not None and max_size_bytes <= 0:
        raise ValueError("max_size_bytes must be positive or None ...")

bool is an int subclass, so `True` passes as a one-byte cap. Every write is
then evicted immediately by the size enforcer:

    cache = InMemoryProgramCache(max_size_bytes=True)
    cache[b"k"] = b"ab"
    len(cache)  ->  0

while its twin `False` is rejected by the same line. A float cap is accepted
too, and a str cap raises `TypeError: '<=' not supported between instances of
'str' and 'int'` rather than the ValueError the constructor documents.

Require a positive int, using the same `isinstance(x, bool) or not
isinstance(x, int)` shape already used by `Host.__new__` and
`checkpoint._check_pid`. The message keeps the word "positive" that the
existing tests match on.

path
----
`FileStreamProgramCache.__init__` does:

    self._root = Path(path) if path is not None else _default_cache_dir()

`Path("")` is `Path(".")`, so an empty path roots the cache in the current
working directory and `__init__` then creates `entries/` and `tmp/` there --
directories a later `clear()` will rmdir. `path=os.environ.get("VAR", "")` is
how a caller lands on it.

The asymmetry is in the same file: `_default_cache_dir()` deliberately treats
an empty `XDG_CACHE_HOME` / `LOCALAPPDATA` as unset. Reject the empty string
rather than defaulting silently, and point at `None` for the default. An
explicit `"."` still works.
@copy-pr-bot

copy-pr-bot Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.core Everything related to the cuda.core module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant