From b99c7784ee0f39231cce6c2988283cb7a201eec2 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Thu, 1 Aug 2019 13:27:02 +0200 Subject: [PATCH 1/5] TST test that abc cache entries are not pickled --- tests/cloudpickle_test.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index ec1d089bb..6574ca666 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -989,6 +989,32 @@ def test_getset_descriptor(self): depickled_descriptor = pickle_depickle(float.real) self.assertIs(depickled_descriptor, float.real) + def test_abc_cache_not_pickled(self): + # cloudpickle issue #302: make sure that cloudpickle does not pickle + # the caches populated during instance/subclass checks of abc.ABCMeta + # instances. + MyClass = abc.ABCMeta('MyClass', (), {}) + + class MyUnrelatedClass: + pass + + class MyRelatedClass: + pass + + MyClass.register(MyRelatedClass) + + assert not issubclass(MyUnrelatedClass, MyClass) + assert issubclass(MyRelatedClass, MyClass) + + s = cloudpickle.dumps(MyClass) + + assert b"MyUnrelatedClass" not in s + assert b"MyRelatedClass" in s + + depickled_class = cloudpickle.loads(s) + assert not issubclass(MyUnrelatedClass, depickled_class) + assert issubclass(MyRelatedClass, depickled_class) + def test_abc(self): @abc.abstractmethod From 3dc61c5a81cc76e048f82adc534bf5cc13cee882 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Thu, 1 Aug 2019 13:29:18 +0200 Subject: [PATCH 2/5] FIX dont pickle abc caches for python<3.7 --- cloudpickle/cloudpickle.py | 21 +++++++++++++++------ cloudpickle/cloudpickle_fast.py | 20 +++++++++++++++----- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py index c92b2eac4..da70bdc36 100644 --- a/cloudpickle/cloudpickle.py +++ b/cloudpickle/cloudpickle.py @@ -42,6 +42,7 @@ """ from __future__ import print_function +import abc import dis from functools import partial import io @@ -617,12 +618,20 @@ def save_dynamic_class(self, obj): clsdict = _extract_class_dict(obj) clsdict.pop('__weakref__', None) - # For ABCMeta in python3.7+, remove _abc_impl as it is not picklable. - # This is a fix which breaks the cache but this only makes the first - # calls to issubclass slower. - if "_abc_impl" in clsdict: - import abc - (registry, _, _, _) = abc._get_dump(obj) + if issubclass(type(obj), abc.ABCMeta): + # If obj is an instance of an ABCMeta subclass, dont pickle the + # cache/negative caches populated during isinstance/issubclass + # checks, but pickle the list of registered subclasses of obj. + clsdict.pop('_abc_cache', None) + clsdict.pop('_abc_negative_cache', None) + clsdict.pop('_abc_negative_cache_version', None) + registry = clsdict.pop('_abc_registry', None) + if registry is None: + # in Python3.7+, the abc caches and registered subclasses of a + # class are bundled into the single _abc_impl attribute + clsdict.pop('_abc_impl', None) + (registry, _, _, _) = abc._get_dump(obj) + clsdict["_abc_impl"] = [subclass_weakref() for subclass_weakref in registry] diff --git a/cloudpickle/cloudpickle_fast.py b/cloudpickle/cloudpickle_fast.py index dfe554d62..29556e475 100644 --- a/cloudpickle/cloudpickle_fast.py +++ b/cloudpickle/cloudpickle_fast.py @@ -136,13 +136,23 @@ def _class_getstate(obj): clsdict = _extract_class_dict(obj) clsdict.pop('__weakref__', None) - # For ABCMeta in python3.7+, remove _abc_impl as it is not picklable. - # This is a fix which breaks the cache but this only makes the first - # calls to issubclass slower. - if "_abc_impl" in clsdict: - (registry, _, _, _) = abc._get_dump(obj) + if issubclass(type(obj), abc.ABCMeta): + # If obj is an instance of an ABCMeta subclass, dont pickle the + # cache/negative caches populated during isinstance/issubclass + # checks, but pickle the list of registered subclasses of obj. + clsdict.pop('_abc_cache', None) + clsdict.pop('_abc_negative_cache', None) + clsdict.pop('_abc_negative_cache_version', None) + registry = clsdict.pop('_abc_registry', None) + if registry is None: + # in Python3.7+, the abc caches and registered subclasses of a + # class are bundled into the single _abc_impl attribute + clsdict.pop('_abc_impl', None) + (registry, _, _, _) = abc._get_dump(obj) + clsdict["_abc_impl"] = [subclass_weakref() for subclass_weakref in registry] + if hasattr(obj, "__slots__"): # pickle string length optimization: member descriptors of obj are # created automatically from obj's __slots__ attribute, no need to From 3b24adaebcdf2e55ec68a63aad83118fcc3c1576 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Thu, 1 Aug 2019 13:31:17 +0200 Subject: [PATCH 3/5] MNT changelog --- CHANGES.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 65f61d451..7869a729a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,10 @@ 1.2.2 ===== +- Dont pickle the abc cache of dynamically defined classes for Python 3.6- + (This was already the case for python3.7+) + ([issue #302](https://github.com/cloudpipe/cloudpickle/issues/302)) + - Fix a bug affecting bound classmethod saving on Python 2. ([issue #288](https://github.com/cloudpipe/cloudpickle/issues/288)) From 746c5a246a853ce5306235ffe4a4e22d30c70ca0 Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Thu, 1 Aug 2019 15:17:03 +0200 Subject: [PATCH 4/5] handle Weakset/set of weakrefs duality --- cloudpickle/cloudpickle.py | 8 ++++++-- cloudpickle/cloudpickle_fast.py | 11 ++--------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py index da70bdc36..a2f7a97f3 100644 --- a/cloudpickle/cloudpickle.py +++ b/cloudpickle/cloudpickle.py @@ -632,8 +632,12 @@ def save_dynamic_class(self, obj): clsdict.pop('_abc_impl', None) (registry, _, _, _) = abc._get_dump(obj) - clsdict["_abc_impl"] = [subclass_weakref() - for subclass_weakref in registry] + clsdict["_abc_impl"] = [subclass_weakref() + for subclass_weakref in registry] + else: + # In the above if clause, registry is a set of weakrefs -- in + # this case, registry is a WeakSet + clsdict["_abc_impl"] = [type_ for type_ in registry] # On PyPy, __doc__ is a readonly attribute, so we need to include it in # the initial skeleton class. This is safe because we know that the diff --git a/cloudpickle/cloudpickle_fast.py b/cloudpickle/cloudpickle_fast.py index 29556e475..5c7057890 100644 --- a/cloudpickle/cloudpickle_fast.py +++ b/cloudpickle/cloudpickle_fast.py @@ -140,15 +140,8 @@ def _class_getstate(obj): # If obj is an instance of an ABCMeta subclass, dont pickle the # cache/negative caches populated during isinstance/issubclass # checks, but pickle the list of registered subclasses of obj. - clsdict.pop('_abc_cache', None) - clsdict.pop('_abc_negative_cache', None) - clsdict.pop('_abc_negative_cache_version', None) - registry = clsdict.pop('_abc_registry', None) - if registry is None: - # in Python3.7+, the abc caches and registered subclasses of a - # class are bundled into the single _abc_impl attribute - clsdict.pop('_abc_impl', None) - (registry, _, _, _) = abc._get_dump(obj) + clsdict.pop('_abc_impl', None) + (registry, _, _, _) = abc._get_dump(obj) clsdict["_abc_impl"] = [subclass_weakref() for subclass_weakref in registry] From 6cac138b75f971707d359c0a6cc64b3952801a9a Mon Sep 17 00:00:00 2001 From: Pierre Glaser Date: Thu, 1 Aug 2019 15:48:01 +0200 Subject: [PATCH 5/5] CLN cosmetics --- cloudpickle/cloudpickle_fast.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cloudpickle/cloudpickle_fast.py b/cloudpickle/cloudpickle_fast.py index 5c7057890..b1311a5f2 100644 --- a/cloudpickle/cloudpickle_fast.py +++ b/cloudpickle/cloudpickle_fast.py @@ -142,7 +142,6 @@ def _class_getstate(obj): # checks, but pickle the list of registered subclasses of obj. clsdict.pop('_abc_impl', None) (registry, _, _, _) = abc._get_dump(obj) - clsdict["_abc_impl"] = [subclass_weakref() for subclass_weakref in registry]