diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index 953fd5c5a27..ba893f39c2f 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -3499,6 +3499,30 @@ class AbstractHookTests(unittest.TestCase): ValueError, 'The reducer just failed'): p.dump(h) + @support.cpython_only + def test_reducer_override_no_reference_cycle(self): + # bpo-39492: reducer_override used to induce a spurious reference cycle + # inside the Pickler object, that could prevent all serialized objects + # from being garbage-collected without explicity invoking gc.collect. + + for proto in range(0, pickle.HIGHEST_PROTOCOL + 1): + with self.subTest(proto=proto): + def f(): + pass + + wr = weakref.ref(f) + + bio = io.BytesIO() + p = self.pickler_class(bio, proto) + p.dump(f) + new_f = pickle.loads(bio.getvalue()) + assert new_f == 5 + + del p + del f + + self.assertIsNone(wr()) + class AbstractDispatchTableTests(unittest.TestCase): diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-01-30-01-14-42.bpo-39492.eTuy0F.rst b/Misc/NEWS.d/next/Core and Builtins/2020-01-30-01-14-42.bpo-39492.eTuy0F.rst new file mode 100644 index 00000000000..6e8b715c463 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2020-01-30-01-14-42.bpo-39492.eTuy0F.rst @@ -0,0 +1 @@ +Fix a reference cycle in the C Pickler that was preventing the garbage collection of deleted, pickled objects. \ No newline at end of file diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 25d5c8da920..fc48d605786 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -4455,12 +4455,13 @@ static int dump(PicklerObject *self, PyObject *obj) { const char stop_op = STOP; + int status = -1; PyObject *tmp; _Py_IDENTIFIER(reducer_override); if (_PyObject_LookupAttrId((PyObject *)self, &PyId_reducer_override, &tmp) < 0) { - return -1; + goto error; } /* Cache the reducer_override method, if it exists. */ if (tmp != NULL) { @@ -4477,7 +4478,7 @@ dump(PicklerObject *self, PyObject *obj) assert(self->proto >= 0 && self->proto < 256); header[1] = (unsigned char)self->proto; if (_Pickler_Write(self, header, 2) < 0) - return -1; + goto error; if (self->proto >= 4) self->framing = 1; } @@ -4485,9 +4486,22 @@ dump(PicklerObject *self, PyObject *obj) if (save(self, obj, 0) < 0 || _Pickler_Write(self, &stop_op, 1) < 0 || _Pickler_CommitFrame(self) < 0) - return -1; + goto error; + + // Success + status = 0; + + error: self->framing = 0; - return 0; + + /* Break the reference cycle we generated at the beginning this function + * call when setting the reducer_override attribute of the Pickler instance + * to a bound method of the same instance. This is important as the Pickler + * instance holds a reference to each object it has pickled (through its + * memo): thus, these objects wont be garbage-collected as long as the + * Pickler itself is not collected. */ + Py_CLEAR(self->reducer_override); + return status; } /*[clinic input]