Skip to content

Commit

Permalink
Don't alter a module when reducing it (#426)
Browse files Browse the repository at this point in the history
  • Loading branch information
pierreglaser committed Jun 19, 2021
1 parent eddd327 commit 2a79b69
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 2 deletions.
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

0 comments on commit 2a79b69

Please sign in to comment.