Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't alter a module when reducing it #426

Merged
merged 5 commits into from Jun 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES.md
Expand Up @@ -4,6 +4,9 @@
dev
===

- Fix a side effect altering dynamic modules at pickling time.
([PR #426](https://github.com/cloudpipe/cloudpickle/pull/426))

- Support for pickling type annotations on Python 3.10 as per [PEP 563](
https://www.python.org/dev/peps/pep-0563/)
([PR #400](https://github.com/cloudpipe/cloudpickle/pull/400))
Expand Down
9 changes: 7 additions & 2 deletions cloudpickle/cloudpickle_fast.py
Expand Up @@ -342,8 +342,13 @@ def _module_reduce(obj):
if _is_importable(obj):
return subimport, (obj.__name__,)
else:
obj.__dict__.pop('__builtins__', None)
return dynamic_subimport, (obj.__name__, vars(obj))
# Some external libraries can populate the "__builtins__" entry of a
# module's `__dict__` with unpicklable objects (see #316). For that
# reason, we do not attempt to pickle the "__builtins__" entry, and
# restore a default value for it at unpickling time.
state = obj.__dict__.copy()
state.pop('__builtins__', None)
return dynamic_subimport, (obj.__name__, state)


def _method_reduce(obj):
Expand Down
6 changes: 6 additions & 0 deletions tests/cloudpickle_test.py
Expand Up @@ -604,6 +604,12 @@ def __reduce__(self):
assert hasattr(depickled_mod.__builtins__, "abs")
assert depickled_mod.f(-1) == 1

# Additional check testing that the issue #425 is fixed: without the
# fix for #425, `mod.f` would not have access to `__builtins__`, and
# thus calling `mod.f(-1)` (which relies on the `abs` builtin) would
# fail.
assert mod.f(-1) == 1

def test_load_dynamic_module_in_grandchild_process(self):
# Make sure that when loaded, a dynamic module preserves its dynamic
# property. Otherwise, this will lead to an ImportError if pickled in
Expand Down