FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

BUG: Mutating returned results leaks into cached copy reused by Pipeline.run by basnijholt · Pull Request #905 · pipefunc/pipefunc · GitHub

BUG: Mutating returned results leaks into cached copy reused by Pipeline.run - #905

Merged
basnijholt merged 2 commits into
mainfrom
fix-cache-result-bug
Jun 10, 2026
Merged

BUG: Mutating returned results leaks into cached copy reused by Pipeline.run#905
basnijholt merged 2 commits into
mainfrom
fix-cache-result-bug

Conversation

basnijholt commented Oct 10, 2025
edited
Loading

Copy link
Copy Markdown
Collaborator

Fixes the bug reported by @MitchellAcoustics in #854 (comment): a ResultDict (or any mutable result) returned by a cached function is stored in the cache by reference, so mutating the returned object — by a downstream function in the same run, or by the user afterwards — silently corrupts the cache entry. The next pipeline.run then returns the mutated object instead of the original result (e.g. ErrorSnapshots stripped to nan), making repeated runs non-idempotent.

The fix

The in-memory caches now store and return deep copies:

  • SimpleCache, LRUCache(shared=False), and HybridCache(shared=False) deep-copy values on put (snapshots the entry before anything can mutate it) and on get (so a consumer of a cache hit cannot corrupt the entry for the next hit). Both sides are required: copy-on-get alone cannot fix this bug, because the object returned to the caller is the stored entry.
  • Shared caches (shared=True, the default) were never affected — they already isolate values via the cloudpickle/manager-proxy round-trip. DiskCache's disk path also already isolates via pickling; its in-memory LRU layer inherits the fix.
  • Opt out with copy=False (e.g. LRUCache(shared=False, copy=False)) for large values that are never mutated. Values that cannot be deep-copied fall back to the original object with a warning.
  • Documented in the caching concept page.

Performance

Copies are made only for the non-shared in-memory caches, once per put and once per hit. deepcopy is cheaper than the cloudpickle round-trip the default shared path already pays, and by definition cheaper than the recomputation the cache avoids. CodSpeed includes a cached-pipeline benchmark (test_calling_pipeline_directly_with_cache) which will quantify it.

Verification

  • The repro test from this PR (tests/map/test_cache_result_dict.py) now passes.
  • New unit tests: put-then-mutate-original, get-then-mutate-returned, and copy=False aliasing opt-out for all three in-memory caches, plus DiskCache's LRU layer.
  • Full suite: 1412 passed; pre-commit run --all-files clean.

Addresses the caching pain point of #902. The separate notebook-staleness issue (cache keys don't include the function body, so editing a function returns stale cached results) is intentionally not part of this PR.

codspeed-hq Bot commented Oct 10, 2025
edited
Loading

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 6 untouched benchmarks


Comparing fix-cache-result-bug (fc3e5aa) with main (53378e5)

Fixes mutations of returned (or later-mutated) results leaking into
cache entries, making repeated `pipeline.run` calls non-idempotent.

- `SimpleCache`, `LRUCache(shared=False)`, and `HybridCache(shared=False)`
  now deep-copy values on `put` and `get` (shared caches already isolate
  values via (de)serialization). `DiskCache`'s in-memory LRU layer
  inherits the fix.
- Opt out with `copy=False` for large never-mutated values.
- Falls back to the original object with a warning if a value cannot be
  deep-copied.

codecov Bot commented Jun 10, 2026
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
pipefunc/_pipefunc.py 100.00% <100.00%> (ø)
pipefunc/_pipeline/_base.py 100.00% <100.00%> (ø)
pipefunc/_run_status.py 100.00% <100.00%> (ø)
pipefunc/_run_status_cli.py 100.00% <100.00%> (ø)
pipefunc/_run_status_heartbeat.py 100.00% <100.00%> (ø)
pipefunc/_utils.py 100.00% <100.00%> (ø)
pipefunc/cache.py 100.00% <100.00%> (ø)
pipefunc/exceptions.py 100.00% <100.00%> (ø)
pipefunc/helpers.py 100.00% <100.00%> (ø)
pipefunc/map/__init__.py 100.00% <100.00%> (ø)
... and 12 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

basnijholt force-pushed the fix-cache-result-bug branch from fc032e6 to fc3e5aa Compare June 10, 2026 15:36

Copy link
Copy Markdown
Contributor

✅ PR Title Formatted Correctly

The title of this PR has been updated to match the correct format. Thank you!

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant


Back | FazBrowse Home | New Git URL