Skip to content

Commit

Permalink
Fix two reference leaks that could occur following an obscure error.
Browse files Browse the repository at this point in the history
Fixes #143
  • Loading branch information
jamadden committed Mar 26, 2020
1 parent e138d04 commit 30f9e02
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 8 deletions.
3 changes: 2 additions & 1 deletion CHANGES.rst
Expand Up @@ -6,7 +6,8 @@

- Fix an overly specific test failure using zope.interface 5. See
`issue 144 <https://github.com/zopefoundation/persistent/issues/144>`_.

- Fix two reference leaks that could theoretically occur as the result
of obscure errors. See `issue 143 <https://github.com/zopefoundation/persistent/issues/143>`_.

4.6.3 (2020-03-18)
------------------
Expand Down
2 changes: 1 addition & 1 deletion persistent/cPersistence.c
Expand Up @@ -1278,9 +1278,9 @@ Per_get_mtime(cPersistentObject *self)
if (!ts_module)
return NULL;
TimeStamp = PyObject_GetAttrString(ts_module, "TimeStamp");
Py_DECREF(ts_module);
if (!TimeStamp)
return NULL;
Py_DECREF(ts_module);
}

#ifdef PY3K
Expand Down
29 changes: 23 additions & 6 deletions persistent/cPickleCache.c
Expand Up @@ -614,15 +614,20 @@ cc_oid_unreferenced(ccobject *self, PyObject *oid)
not release the global interpreter lock until this is
complete. */

PyObject *dead_pers_obj;
cPersistentObject *dead_pers_obj;
ccobject* dead_pers_obj_ref_to_self;

/* If the cache has been cleared by GC, data will be NULL. */
if (!self->data)
return;

dead_pers_obj = PyDict_GetItem(self->data, oid);
dead_pers_obj = (cPersistentObject*)PyDict_GetItem(self->data, oid);
assert(dead_pers_obj);
assert(dead_pers_obj->ob_refcnt == 0);

dead_pers_obj_ref_to_self = (ccobject*)dead_pers_obj->cache;
assert(dead_pers_obj_ref_to_self == self);

/* Need to be very hairy here because a dictionary is about
to decref an already deleted object.
*/
Expand All @@ -636,17 +641,29 @@ cc_oid_unreferenced(ccobject *self, PyObject *oid)
Py_INCREF(dead_pers_obj);

if (PyDict_DelItem(self->data, oid) < 0)
return;
{
/* Almost ignore errors if it wasn't already present (somehow;
that shouldn't be possible since we literally just got it out
of this dict and we're holding the GIL and not making any
calls that could cause a greenlet switch so the state of the
dictionary should not change). We still need to finish the cleanup.
Just write an unraisable error (like an exception from __del__,
because that's basically what this is).
*/
PyErr_WriteUnraisable((PyObject*)dead_pers_obj);
PyErr_Clear();
}
/* Now remove the dead object's reference to self. Note that this could
cause self to be dealloced.
*/
Py_DECREF((ccobject *)((cPersistentObject *)dead_pers_obj)->cache);
((cPersistentObject *)dead_pers_obj)->cache = NULL;
Py_DECREF(dead_pers_obj_ref_to_self);
dead_pers_obj->cache = NULL;

assert(dead_pers_obj->ob_refcnt == 1);

/* Don't DECREF the object, because this function is called from
the object's dealloc function. If the refcnt reaches zero, it
the object's dealloc function. If the refcnt reaches zero (again), it
will all be invoked recursively.
*/
}
Expand Down

0 comments on commit 30f9e02

Please sign in to comment.