diff --git a/CHANGES.rst b/CHANGES.rst index 2308c08..c8b39c9 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,7 +6,8 @@ - Fix an overly specific test failure using zope.interface 5. See `issue 144 `_. - +- Fix two reference leaks that could theoretically occur as the result + of obscure errors. See `issue 143 `_. 4.6.3 (2020-03-18) ------------------ diff --git a/persistent/cPersistence.c b/persistent/cPersistence.c index 7bcc39f..049288e 100644 --- a/persistent/cPersistence.c +++ b/persistent/cPersistence.c @@ -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 diff --git a/persistent/cPickleCache.c b/persistent/cPickleCache.c index c85c575..68e1a22 100644 --- a/persistent/cPickleCache.c +++ b/persistent/cPickleCache.c @@ -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. */ @@ -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. */ }