Skip to content

Match upstream cloudpickle in the behaviors found by a line-by-line audit - #3

Merged
HyukjinKwon merged 3 commits into
mainfrom
audit-upstream-parity
Nov 5, 2026
Merged

HyukjinKwon merged 3 commits into
mainfrom
audit-upstream-parity

Conversation

@HyukjinKwon

@HyukjinKwon HyukjinKwon commented Nov 5, 2026 •

Copy link
Copy Markdown
Owner

What

I compared every line of upstream's cloudpickle/cloudpickle.py (commit 4af5936) with the Rust implementation. No function, branch or payload difference was missing. This PR fixes the behavior differences the comparison found.

Globals are read at call time, like upstream

Upstream's functions read their module's globals on every call: its state (_PICKLE_BY_VALUE_MODULES, the class trackers, _extract_code_globals_cache, …), its own functions, and the modules it imported. They also read the standard-library attributes they use (types.FunctionType, uuid.uuid4, sys.modules, pickle.PicklingError, warnings.warn, …). Replacing one of them, for example with monkeypatch, therefore changes upstream's behavior. The port bound them at import, so the same replacement had no effect.

Now each Rust function first checks that the globals the upstream function reads still hold their initial values, at one dictionary lookup each. If one was replaced, the call runs the upstream function itself. It is compiled on demand from a byte-identical copy of upstream's cloudpickle.py (src/upstream/cloudpickle.py), with the namespace of cloudpickle_rust.cloudpickle as its globals (src/fallback.rs). The per-function dependency lists come from upstream's bytecode.

_code_reduce reads a single global, types.CodeType, when it returns. It looks it up at that point instead of running upstream's code, so DYNAMIC_CODE_FILENAME (#2) applies in every case.

The functions are Python functions, like upstream's

The module's 65 functions and the Pickler methods are now Python functions that call their Rust implementation (src/functions.rs). They have upstream's signatures (now including _make_cell(value=_empty_cell_value)), docstrings, __qualname__, __module__ and __globals__. The docstrings are compiled by the running interpreter, so they also match Python 3.13's docstring dedenting. They come from src/upstream_docs.rs, which dev/generate_upstream_docs.py generates from the bundled source. As a result:

  • isinstance(f, types.FunctionType) matches upstream. The pickler therefore routes the module's own reconstructors through _function_reduce, so Pickler subclasses and replaced _should_pickle_by_reference see the same calls as upstream.
  • Warnings raised while pickling are attributed to the module, as upstream's are to cloudpickle.cloudpickle.

The exception is Pickler.reducer_override: the C pickler calls it for every object, so it stays a native method descriptor.

Other fixes

  • _extract_code_globals_cache is a weakref.WeakKeyDictionary, and _empty_cell_value is an instance of a Python class, both like upstream's.
  • Error messages match upstream's:
    • _getattribute on 3.12 and 3.13;
    • unpacking errors in _function_setstate and _class_setstate on 3.14;
    • _file_reduce for a non-string mode or name (% formatting);
    • register_pickle_by_value and unregister_pickle_by_value (format()).
  • _walk_global_ops, _extract_code_globals and _find_imported_submodules accept the same non-code inputs as upstream. For example, _walk_global_ops(func) returns upstream's lazy generator.
  • PyPy: the module's functions are saved by upstream's own save_function, and the extra reducer_override the port added is removed.
  • Docs: the README compatibility section lists the remaining implementation differences, and LICENSE is updated. The CHANGES.md entries go in the 0.2.0 section, next to Add DYNAMIC_CODE_FILENAME to keep the filenames of the code pickled by value #2's. The README benchmark table is updated too: pickling functions is about 10% slower than in 0.1.0, which is the cost of these semantics, while instance-heavy data is unchanged.

Not ported: upstream's label-triggered downstream CI jobs (distributed, joblib, loky, ray). Its MANIFEST.in, .git-blame-ignore-revs and dev/make-distribution.sh aren't needed here.

Verification

Rebased on main after #2 was merged (conflicts resolved: module_global replaced by fallback::load_global).

  • Ran the full suite locally on CPython 3.8–3.14, 3.13t and 3.14t. All 278 upstream tests have the same outcome as before, and every port-specific test passes.
  • On PyPy 7.3.23 and 8.0.1, all upstream tests have the same outcome. The only change is test_walk_global_ops_matches_dis, which now passes on 7.3.23 where the old baseline failed.
  • New differential tests compare against upstream cloudpickle on the same interpreter:
    • replacing each internal function with a counting spy gives the same call counts and payloads;
    • replacing each imported module with a recording proxy gives the same attribute-access sequence;
    • reassigning the module state behaves the same;
    • functions and methods have the same type, signature, docstring and globals;
    • the error messages and the bundled source are identical.
  • pre-commit (all files), clippy -D warnings, and the sdist (which includes the new sources).

This pull request and its description were written by Isaac.

HyukjinKwon and others added 3 commits November 5, 2026 18:29
…udit

A line-by-line review of upstream's cloudpickle/cloudpickle.py against the
Rust implementation found no missing function or code path, but found these
differences, now fixed:

- Upstream's functions read the globals of their module and the attributes
  of the standard library modules they use at call time, so replacing them
  (e.g. with monkeypatch) changes their behavior. Each Rust function now
  checks that the globals read by the upstream function still hold their
  initial values, and otherwise runs the upstream function, compiled from a
  bundled copy of upstream's cloudpickle.py (src/upstream/cloudpickle.py)
  with the namespace of cloudpickle_rust.cloudpickle as its globals.
- The functions of the module and the methods of Pickler (except
  reducer_override, which the C pickler calls for every object) are Python
  functions calling their Rust implementation, with upstream's signatures,
  docstrings and globals: like upstream's, they are types.FunctionType
  instances (Pickler subclasses see the same _function_reduce calls) and
  warnings raised while pickling are attributed to the module.
- _extract_code_globals_cache is a weakref.WeakKeyDictionary and
  _empty_cell_value an instance of a Python class, like upstream's.
- Same error messages as upstream for _getattribute on Python 3.12/3.13, for
  iterable unpacking on Python 3.14, for file objects with a non-string mode
  or name, and in register/unregister_pickle_by_value (format()).
- _walk_global_ops, _extract_code_globals and _find_imported_submodules
  handle other objects than code objects like upstream.
- On PyPy, the functions of the module are saved by save_function like
  upstream's (the extra reducer_override is removed).

Pickling functions and classes is about 10% slower than with 0.1.0 (the
cost of these semantics); the README benchmark is updated.

New tests compare, with upstream cloudpickle, the behavior when each global
of the module or each imported module is replaced, the functions
(signatures, docstrings, globals), the error messages and the bundled source.

Co-authored-by: Isaac <no-reply@databricks.com>
Windows formats the addresses in the repr of objects with uppercase
hexadecimal digits.

Co-authored-by: Isaac <no-reply@databricks.com>
dev/generate_upstream_docs.py extracts the docstrings of the functions and
of the Pickler methods from src/upstream/cloudpickle.py with the ast module
(the values of their literals, whatever the version of Python).

Co-authored-by: Isaac <no-reply@databricks.com>
@HyukjinKwon
HyukjinKwon force-pushed the audit-upstream-parity branch from beed6a3 to 2bc066f Compare November 5, 2026 09:30
@HyukjinKwon
HyukjinKwon merged commit 752e486 into main Nov 5, 2026
64 checks passed
@HyukjinKwon
HyukjinKwon deleted the audit-upstream-parity branch November 5, 2026 09:45
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