diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py index c92b2eac4..cd6bd2faa 100644 --- a/cloudpickle/cloudpickle.py +++ b/cloudpickle/cloudpickle.py @@ -642,10 +642,34 @@ def save_dynamic_class(self, obj): for k in obj.__slots__: clsdict.pop(k, None) - # If type overrides __dict__ as a property, include it in the type - # kwargs. In Python 2, we can't set this attribute after construction. + # A class __dict__ is part of the class state. At unpickling time, it + # must be *initialized* (in an empty state) during class creation and + # updated during class re-hydratation. + # However, a class __dict__ is read-only, and does not support direct + # item assignement. Instead, the way to update a class __dict__ is to + # call setattr(klass, k, v) on the underlying class, which has the same + # effect. + # There is one corner case: if the __dict__ class has itself a + # "__dict__" key (this means that the class likely overrides the + # __dict__ property of its instances), setattr(klass, "__dict__", v) will try + # to modify the read-only class __dict__ instead, and fail. As a + # result, if it exists, the class __dict__ must contain its __dict__ + # item when it is initialized and fed to the class reconstructor. __dict__ = clsdict.pop('__dict__', None) - if isinstance(__dict__, property): + + # As __dict__ is part of obj's reconstructor args, __dict__ will be + # saved before obj is memoized. Thus, we must make sure that cyclic + # references between __dict__ and obj will not trigger infinite + # recursion, i.e that __dict__ is memoized before its populated with + # obj. If __dict__ is a dict, references to obj inside __dict__ are + # safe. But if __dict__ is not overriden, it is a getset_descriptor + # that contains an unsafe reference to obj, and we must not save it. + # Because non-overriden __dict__ attributes will be generated + # automatically at class reconstruction anyways, no information is + # lost. + if (__dict__ is not None and + not isinstance(__dict__, types.GetSetDescriptorType) and + getattr(__dict__, '__objclass__', None) is not obj): type_kwargs['__dict__'] = __dict__ save = self.save diff --git a/cloudpickle/cloudpickle_fast.py b/cloudpickle/cloudpickle_fast.py index dfe554d62..5e4a20a31 100644 --- a/cloudpickle/cloudpickle_fast.py +++ b/cloudpickle/cloudpickle_fast.py @@ -72,7 +72,8 @@ def _class_getnewargs(obj): type_kwargs["__slots__"] = obj.__slots__ __dict__ = obj.__dict__.get('__dict__', None) - if isinstance(__dict__, property): + if (not isinstance(__dict__, types.GetSetDescriptorType) and + getattr(__dict__, '__objclass__', None) is not obj): type_kwargs['__dict__'] = __dict__ return (type(obj), obj.__name__, obj.__bases__, type_kwargs, @@ -153,7 +154,7 @@ def _class_getstate(obj): for k in obj.__slots__: clsdict.pop(k, None) - clsdict.pop('__dict__', None) # unpicklable property object + clsdict.pop('__dict__', None) # specified in class reconstruction return (clsdict, {}) diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py index ec1d089bb..2a3df1310 100644 --- a/tests/cloudpickle_test.py +++ b/tests/cloudpickle_test.py @@ -1867,6 +1867,31 @@ def __getattr__(self, name): with pytest.raises(pickle.PicklingError, match='recursion'): cloudpickle.dumps(a) + def test___dict__attribute_not_dropped_during_pickling(self): + # Test https://github.com/cloudpipe/cloudpickle/issues/282. cloudpickle + # used to drop __dict__ attributes of classes at pickling time. + pickle_filename= os.path.join(self.tmpdir, 'class_with_dict.pkl') + _dict = {'some_attribute': 1} + class A(object): + __dict__ = _dict + a = A() + self.assertEqual(a.__dict__, _dict) + + with open(pickle_filename, "wb") as f: + cloudpickle.dump(a, f, protocol=self.protocol) + + # Check that the dynamic class defintion is fully reconstructed in a + # new Python subprocess and check the assertion that the content of the + # __dict__ attribute has the expected content there. Testing this in + # the main process is not enough because the existing class definition + # could have been reused directly. + child_process_script = """ + import pickle + with open(r"{filename}", "rb") as f: + depickled_a = pickle.load(f) + assert depickled_a.__dict__ == {_dict}, depickled_a.__dict__ + """.format(filename=pickle_filename, _dict=_dict) + assert_run_python_script(textwrap.dedent(child_process_script)) class Protocol2CloudPickleTest(CloudPickleTest):