Skip to content

Fix pickling of cached_property descriptors before Python 3.12 - #617

Open
arindamsikder wants to merge 1 commit into
cloudpipe:masterfrom
arindamsikder:fix/cached-property-serialization
Open

arindamsikder wants to merge 1 commit into
cloudpipe:masterfrom
arindamsikder:fix/cached-property-serialization

Conversation

@arindamsikder

Copy link
Copy Markdown

Summary

Support serializing classes, instances and standalone functools.cached_property descriptors on Python versions before 3.12.

Problem

On Python 3.8–3.11, cached_property contains an internal RLock. Serializing a dynamically defined class or instance with that descriptor fails with TypeError: cannot pickle '_thread.RLock' object. Python 3.12+ removed the internal lock and does not have this failure.

Solution

Add an exact-type reducer that reconstructs the descriptor from its getter, lets the constructor create a new internal lock where needed, and restores the descriptor's remaining state. Preserve the assigned attribute name, documentation, custom attributes, repeated references and cycles through descriptor state. On Python 3.12+, preserve a user-defined attribute named lock.

The descriptor does not store cached results; those remain in the instance dictionary and retain the normal instance-serialization behavior. No general lock serializer or arbitrary descriptor-subclass support is added.

Prior work: this follows pierreglaser's minimal-reducer suggestion in #438 and the reducer approach in @charan-rathore's author-closed #605. That PR was not merged; its contribution is acknowledged rather than presented as a new design. The tests here additionally exercise self-referential descriptor state and evaluate reconstructed classes in a fresh interpreter.

Testing

Executed in credential-free, offline Bubblewrap sandboxes on Linux. Private host paths and inherited credentials were absent, outbound networking was unavailable, and imports resolved to the tested checkout. Each interpreter used its own dependency layer.

Interpreter commands inside those sandboxes:

python -m pytest tests/cloudpickle_test.py -q -k cached_property \
  -p no:cacheprovider --basetemp=/tmp/pytest
python -m pytest tests -lv --maxfail=5 \
  -p no:cacheprovider --basetemp=/tmp/pytest
  • Python 3.11.15: the initial descriptor regression failed for both default and protocol 2 before the fix. All eight final regression cases fail with the original runtime source overmounted and pass with the fix. Full baseline: 273 passed, 3 skipped, 2 xfailed; final: 281 passed, 3 skipped, 2 xfailed.
  • Python 3.13.16: full baseline: 265 passed, 11 skipped, 2 xfailed; final: 273 passed, 11 skipped, 2 xfailed, with the same two existing itertools deprecation warnings. The first unchanged baseline run exceeded an existing notebook-startup timing assertion (10.81 seconds versus a 10-second bound); one unchanged rerun passed. No timeout threshold, skip or assertion was changed.
  • Regression coverage includes fresh-interpreter class reconstruction, instances before/after caching, deletion/recomputation, bound and unbound descriptors, metadata, identity and self-reference. Existing property/classmethod/staticmethod tests ran in the full suites.
  • Additional isolated probes passed on both runtimes and both protocols for getters closing over their owner or descriptor. On 3.11, a descriptor serialized while its original lock was held reconstructed an independently usable lock; the original lock remained unchanged.

Pinned quality gates from .pre-commit-config.yaml (Black 25.9.0, Ruff 0.14.3, pre-commit-hooks 6.0.0) ran offline using public hook checkouts and a wheel cache:

python -m pre_commit run --files *
python -m pre_commit run --files CHANGES.md cloudpickle/cloudpickle.py tests/cloudpickle_test.py
python -m pre_commit run --all-files
python -m build --no-isolation --outdir dist-reviewed
python -m twine check --strict dist-reviewed/*
  • The CI's --files * invocation passes on baseline and final, but skips Black/Ruff because it supplies only root entries. The explicit affected-file invocation runs Black/Ruff and passes on both.
  • The stronger --all-files invocation is not clean: Black reformats the unchanged tests/cloudpickle_ipykernel_test.py identically on baseline and final. Other hooks pass. That unrelated formatting is excluded from this PR; the affected files require no changes.
  • Baseline and final sdist/wheel builds (including wheel-from-sdist) and strict Twine metadata checks pass. Both archives contain the exact reviewed runtime bytes.
  • Independent automated diff review and added-line security review passed. git diff --check passed.

Other Python versions, PyPy, native Windows/macOS and optional downstream integration jobs were not run locally. No separate type-check or documentation-build gate is configured for these changed files. This remains same-version serialization, not cross-version or long-term pickle compatibility.

Related Issue

Fixes #438.
Related prior implementation: #605.

Automation disclosure: implemented, tested and independently reviewed by AI agents (Hermes DEV / GPT-6-Astra) on behalf of the account owner. No human pre-review is claimed.

Recreate pre-3.12 descriptor locks while preserving descriptor state and instance caches. Regression coverage includes default and protocol 2 roundtrips.

Based on the reducer approach in charan-rathore's closed PR cloudpipe#605 and pierreglaser's guidance in cloudpipe#438. AI-assisted implementation and independent automated review; no human pre-review claimed.
@charan-rathore

Copy link
Copy Markdown

Thanks for picking this up, and for the generous acknowledgment. The minimal-reducer shape from #605 held up well in my testing, so it's good to see it carried forward here with the self-referential descriptor state and fresh-interpreter evaluation covered. Happy to dig out any context from the closed attempt if it would help the review.

Prepared with AI assistance (Instinct assisted).

This branch has not been deployed

No deployments
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.

Classes with cached_property can't be cloudpickled

2 participants