Skip to content

Commit

Permalink
Fix possibility of rare crash during dealloc. Fixes #66
Browse files Browse the repository at this point in the history
  • Loading branch information
KIMURA Chikahiro authored and jamadden committed Sep 15, 2017
1 parent 740e1d4 commit 4d2dcc9
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 6 deletions.
3 changes: 2 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
4.2.5 (unreleased)
------------------

- Nothing changed yet.
- Fix the possibility of a rare crash in the C extension when
deallocating items. See https://github.com/zopefoundation/persistent/issues/66


4.2.4.2 (2017-04-23)
Expand Down
1 change: 1 addition & 0 deletions persistent/cPersistence.c
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,7 @@ Per__getstate__(cPersistentObject *self)
static void
Per_dealloc(cPersistentObject *self)
{
PyObject_GC_UnTrack((PyObject *)self);
if (self->state >= 0)
{
/* If the cache has been cleared, then a non-ghost object
Expand Down
11 changes: 6 additions & 5 deletions persistent/cPickleCache.c
Original file line number Diff line number Diff line change
Expand Up @@ -206,22 +206,22 @@ scan_gc_items(ccobject *self, int target, Py_ssize_t target_bytes)
{
assert(self->ring_lock);
assert(here != &self->ring_home);

/* At this point we know that the ring only contains nodes
from persistent objects, plus our own home node. We know
this because the ring lock is held. We can safely assume
the current ring node is a persistent object now we know it
is not the home */
object = OBJECT_FROM_RING(self, here);

if (object->state == cPersistent_UPTODATE_STATE)
if (object->state == cPersistent_UPTODATE_STATE)
{
CPersistentRing placeholder;
PyObject *method;
PyObject *temp;
int error_occurred = 0;
/* deactivate it. This is the main memory saver. */

/* Add a placeholder, a dummy node in the ring. We need
to do this to mark our position in the ring. It is
possible that the PyObject_GetAttr() call below will
Expand All @@ -237,7 +237,7 @@ scan_gc_items(ccobject *self, int target, Py_ssize_t target_bytes)
method = PyObject_GetAttr((PyObject *)object, py__p_deactivate);
if (method == NULL)
error_occurred = 1;
else
else
{
temp = PyObject_CallObject(method, NULL);
Py_DECREF(method);
Expand Down Expand Up @@ -891,6 +891,7 @@ cc_init(ccobject *self, PyObject *args, PyObject *kwds)
static void
cc_dealloc(ccobject *self)
{
PyObject_GC_UnTrack((PyObject *)self);
Py_XDECREF(self->data);
Py_XDECREF(self->jar);
PyObject_GC_Del(self);
Expand Down Expand Up @@ -1058,7 +1059,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
PyErr_Format(PyExc_TypeError,
"Cached object oid must be bytes, not a %s",
oid->ob_type->tp_name);

return -1;
}

Expand Down

0 comments on commit 4d2dcc9

Please sign in to comment.