From 8a1578be2ab79200563e589e5ac643be82498463 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Tue, 15 Jun 2021 18:35:59 +0100 Subject: [PATCH 1/5] TST trigger the bug of #425 in a test --- tests/cloudpickle_test.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index 845f27962..2be6c7009 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -604,6 +604,9 @@ def __reduce__(self): assert hasattr(depickled_mod.__builtins__, "abs") assert depickled_mod.f(-1) == 1 + # Additional check testing that the issue #425 is fixed + 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 From 1de1c4fb059de4d2f9bb85f49e6fa69d9ab8f020 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Tue, 15 Jun 2021 18:41:23 +0100 Subject: [PATCH 2/5] FIX remove the __builtins__ from a copy of the module --- cloudpickle/cloudpickle_fast.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cloudpickle/cloudpickle_fast.py b/cloudpickle/cloudpickle_fast.py index 46a9540ec..fb9d28ac3 100644 --- a/cloudpickle/cloudpickle_fast.py +++ b/cloudpickle/cloudpickle_fast.py @@ -342,8 +342,9 @@ 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)) + state = obj.__dict__.copy() + state.pop('__builtins__', None) + return dynamic_subimport, (obj.__name__, state) def _method_reduce(obj): From 05ebfd3396df029c5eaa3374bcfd47c42618aa03 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Tue, 15 Jun 2021 18:48:00 +0100 Subject: [PATCH 3/5] MNT document change inside CHANGES.md --- CHANGES.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index c9b3d9384..91f57174c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -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)) From 8471ca36eeda9c369c3dc936820b3ddd8e5749e5 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Sat, 19 Jun 2021 21:32:06 +0100 Subject: [PATCH 4/5] DOC document the new testing code more explicitly --- tests/cloudpickle_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index 2be6c7009..3851ba99a 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -604,7 +604,10 @@ def __reduce__(self): assert hasattr(depickled_mod.__builtins__, "abs") assert depickled_mod.f(-1) == 1 - # Additional check testing that the issue #425 is fixed + # 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): From 4d45d6ee72482d6c89c2eac02345e76ec1097d18 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Sat, 19 Jun 2021 21:39:19 +0100 Subject: [PATCH 5/5] DOC document __builtins__ of modules are not pickled --- cloudpickle/cloudpickle_fast.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cloudpickle/cloudpickle_fast.py b/cloudpickle/cloudpickle_fast.py index fb9d28ac3..be1bb83c0 100644 --- a/cloudpickle/cloudpickle_fast.py +++ b/cloudpickle/cloudpickle_fast.py @@ -342,6 +342,10 @@ def _module_reduce(obj): if _is_importable(obj): return subimport, (obj.__name__,) else: + # 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)