Commit e5567b6e authored by Jeremy Hylton's avatar Jeremy Hylton

Fix memory leak involving persistent objects and caches.

Problem reported by Ulla Theiss and diagnosed by Shane Hathaway.  The
long and short of it is that persistent objects refer to their
connection which refers to its cache which refers to the persistent
objects.  We can't let GC find the cycle because persistent objects
are extension class objects, and, thus, don't participate in GC.

Add an explicit clear() method that removes non-ghost objects from the
ring without going to all the trouble of ghosting them.  At some
point, they'll just get deallocated (which will ghostify them).

A cleared cache has non-ghost objects in the dict that are not in the
ring, so we need to add some checks for whether the object is actually
in the ring.
parent cb8b868e
...@@ -14,7 +14,7 @@ ...@@ -14,7 +14,7 @@
static char cPersistence_doc_string[] = static char cPersistence_doc_string[] =
"Defines Persistent mixin class for persistent objects.\n" "Defines Persistent mixin class for persistent objects.\n"
"\n" "\n"
"$Id: cPersistence.c,v 1.68 2003/04/02 16:50:49 jeremy Exp $\n"; "$Id: cPersistence.c,v 1.69 2003/04/23 20:05:51 jeremy Exp $\n";
#include "cPersistence.h" #include "cPersistence.h"
...@@ -154,7 +154,7 @@ static void ...@@ -154,7 +154,7 @@ static void
accessed(cPersistentObject *self) accessed(cPersistentObject *self)
{ {
/* Do nothing unless the object is in a cache and not a ghost. */ /* Do nothing unless the object is in a cache and not a ghost. */
if (self->cache && self->state >= 0) { if (self->cache && self->state >= 0 && self->ring.next) {
CPersistentRing *home = &self->cache->ring_home; CPersistentRing *home = &self->cache->ring_home;
self->ring.prev->next = self->ring.next; self->ring.prev->next = self->ring.next;
self->ring.next->prev = self->ring.prev; self->ring.next->prev = self->ring.prev;
...@@ -176,6 +176,13 @@ ghostify(cPersistentObject *self) ...@@ -176,6 +176,13 @@ ghostify(cPersistentObject *self)
self->state = cPersistent_GHOST_STATE; self->state = cPersistent_GHOST_STATE;
return; return;
} }
/* If the cache has been cleared, then a non-ghost object
isn't in the ring any longer.
*/
if (self->ring.next == NULL)
return;
/* if we're ghostifying an object, we better have some non-ghosts */ /* if we're ghostifying an object, we better have some non-ghosts */
assert(self->cache->non_ghost_count > 0); assert(self->cache->non_ghost_count > 0);
......
...@@ -90,7 +90,7 @@ process must skip such objects, rather than deactivating them. ...@@ -90,7 +90,7 @@ process must skip such objects, rather than deactivating them.
static char cPickleCache_doc_string[] = static char cPickleCache_doc_string[] =
"Defines the PickleCache used by ZODB Connection objects.\n" "Defines the PickleCache used by ZODB Connection objects.\n"
"\n" "\n"
"$Id: cPickleCache.c,v 1.81 2003/04/08 15:55:44 jeremy Exp $\n"; "$Id: cPickleCache.c,v 1.82 2003/04/23 20:05:51 jeremy Exp $\n";
#define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;} #define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;}
#define UNLESS(E) if(!(E)) #define UNLESS(E) if(!(E))
...@@ -104,6 +104,7 @@ static char cPickleCache_doc_string[] = ...@@ -104,6 +104,7 @@ static char cPickleCache_doc_string[] =
#undef Py_FindMethod #undef Py_FindMethod
static PyObject *py__p_oid, *py_reload, *py__p_jar, *py__p_changed; static PyObject *py__p_oid, *py_reload, *py__p_jar, *py__p_changed;
static cPersistenceCAPIstruct *capi;
/* Do we want 'engine noise'.... abstract debugging output useful for /* Do we want 'engine noise'.... abstract debugging output useful for
visualizing cache behavior */ visualizing cache behavior */
...@@ -462,6 +463,46 @@ cc_lru_items(ccobject *self, PyObject *args) ...@@ -462,6 +463,46 @@ cc_lru_items(ccobject *self, PyObject *args)
return l; return l;
} }
/* Be very careful about calling clear().
It removes all non-ghost objects from the ring without otherwise
removing them from the cache. The method should only be called
after the cache is no longer in use.
*/
static PyObject *
cc_clear(ccobject *self, PyObject *args)
{
CPersistentRing *here;
if (!PyArg_ParseTuple(args, ":clear"))
return NULL;
if (self->ring_lock) {
/* When the ring lock is held, we have no way of know which
ring nodes belong to persistent objects, and which a
placeholders. */
PyErr_SetString(PyExc_ValueError,
".lru_items() is unavailable during garbage collection");
return NULL;
}
self->ring_lock = 1;
while ((here = self->ring_home.next) != & self->ring_home) {
cPersistentObject *o = OBJECT_FROM_RING(self, here, "clear");
self->non_ghost_count--;
o->ring.next->prev = &self->ring_home;
self->ring_home.next = o->ring.next;
o->ring.next = NULL;
o->ring.prev = NULL;
Py_DECREF(o);
}
self->ring_lock = 0;
Py_INCREF(Py_None);
return Py_None;
}
static int static int
cc_oid_unreferenced(ccobject *self, PyObject *oid) cc_oid_unreferenced(ccobject *self, PyObject *oid)
{ {
...@@ -570,6 +611,8 @@ static struct PyMethodDef cc_methods[] = { ...@@ -570,6 +611,8 @@ static struct PyMethodDef cc_methods[] = {
"get(key [, default]) -- get an item, or a default"}, "get(key [, default]) -- get an item, or a default"},
{"ringlen", (PyCFunction)cc_ringlen, METH_VARARGS, {"ringlen", (PyCFunction)cc_ringlen, METH_VARARGS,
"ringlen() -- Returns number of non-ghost items in cache."}, "ringlen() -- Returns number of non-ghost items in cache."},
{"clear", (PyCFunction)cc_clear, METH_VARARGS,
"clear() -- remove all objects from the cache"},
{NULL, NULL} /* sentinel */ {NULL, NULL} /* sentinel */
}; };
...@@ -923,7 +966,6 @@ void ...@@ -923,7 +966,6 @@ void
initcPickleCache(void) initcPickleCache(void)
{ {
PyObject *m, *d; PyObject *m, *d;
cPersistenceCAPIstruct *capi;
Cctype.ob_type = &PyType_Type; Cctype.ob_type = &PyType_Type;
......
...@@ -13,7 +13,7 @@ ...@@ -13,7 +13,7 @@
############################################################################## ##############################################################################
"""Database connection support """Database connection support
$Id: Connection.py,v 1.89 2003/04/22 18:04:37 jeremy Exp $""" $Id: Connection.py,v 1.90 2003/04/23 20:05:51 jeremy Exp $"""
from __future__ import nested_scopes from __future__ import nested_scopes
...@@ -131,12 +131,15 @@ class Connection(ExportImport.ExportImport): ...@@ -131,12 +131,15 @@ class Connection(ExportImport.ExportImport):
return '<Connection at %08x%s>' % (id(self), ver) return '<Connection at %08x%s>' % (id(self), ver)
def _breakcr(self): def _breakcr(self):
try: del self._cache # Persistent objects and the cache don't participate in GC.
except: pass # Explicitly remove references from the connection to its
try: del self._incrgc # cache and to the root object, because they refer back to the
except: pass # connection.
try: del self.cacheGC self._cache.clear()
except: pass self._cache = None
self._incrgc = None
self.cacheGC = None
self._root_ = None
def __getitem__(self, oid, tt=type(())): def __getitem__(self, oid, tt=type(())):
obj = self._cache.get(oid, None) obj = self._cache.get(oid, None)
......
...@@ -13,8 +13,8 @@ ...@@ -13,8 +13,8 @@
############################################################################## ##############################################################################
"""Database objects """Database objects
$Id: DB.py,v 1.49 2003/04/22 18:04:37 jeremy Exp $""" $Id: DB.py,v 1.50 2003/04/23 20:05:51 jeremy Exp $"""
__version__='$Revision: 1.49 $'[11:-2] __version__='$Revision: 1.50 $'[11:-2]
import cPickle, cStringIO, sys, POSException, UndoLogCompatible import cPickle, cStringIO, sys, POSException, UndoLogCompatible
from Connection import Connection from Connection import Connection
...@@ -255,6 +255,9 @@ class DB(UndoLogCompatible.UndoLogCompatible): ...@@ -255,6 +255,9 @@ class DB(UndoLogCompatible.UndoLogCompatible):
def close(self): def close(self):
self._storage.close() self._storage.close()
for x, allocated in self._pools[1]:
for c in allocated:
c._breakcr()
def commitVersion(self, source, destination='', transaction=None): def commitVersion(self, source, destination='', transaction=None):
if transaction is None: if transaction is None:
......
...@@ -14,7 +14,7 @@ ...@@ -14,7 +14,7 @@
static char cPersistence_doc_string[] = static char cPersistence_doc_string[] =
"Defines Persistent mixin class for persistent objects.\n" "Defines Persistent mixin class for persistent objects.\n"
"\n" "\n"
"$Id: cPersistence.c,v 1.68 2003/04/02 16:50:49 jeremy Exp $\n"; "$Id: cPersistence.c,v 1.69 2003/04/23 20:05:51 jeremy Exp $\n";
#include "cPersistence.h" #include "cPersistence.h"
...@@ -154,7 +154,7 @@ static void ...@@ -154,7 +154,7 @@ static void
accessed(cPersistentObject *self) accessed(cPersistentObject *self)
{ {
/* Do nothing unless the object is in a cache and not a ghost. */ /* Do nothing unless the object is in a cache and not a ghost. */
if (self->cache && self->state >= 0) { if (self->cache && self->state >= 0 && self->ring.next) {
CPersistentRing *home = &self->cache->ring_home; CPersistentRing *home = &self->cache->ring_home;
self->ring.prev->next = self->ring.next; self->ring.prev->next = self->ring.next;
self->ring.next->prev = self->ring.prev; self->ring.next->prev = self->ring.prev;
...@@ -176,6 +176,13 @@ ghostify(cPersistentObject *self) ...@@ -176,6 +176,13 @@ ghostify(cPersistentObject *self)
self->state = cPersistent_GHOST_STATE; self->state = cPersistent_GHOST_STATE;
return; return;
} }
/* If the cache has been cleared, then a non-ghost object
isn't in the ring any longer.
*/
if (self->ring.next == NULL)
return;
/* if we're ghostifying an object, we better have some non-ghosts */ /* if we're ghostifying an object, we better have some non-ghosts */
assert(self->cache->non_ghost_count > 0); assert(self->cache->non_ghost_count > 0);
......
...@@ -90,7 +90,7 @@ process must skip such objects, rather than deactivating them. ...@@ -90,7 +90,7 @@ process must skip such objects, rather than deactivating them.
static char cPickleCache_doc_string[] = static char cPickleCache_doc_string[] =
"Defines the PickleCache used by ZODB Connection objects.\n" "Defines the PickleCache used by ZODB Connection objects.\n"
"\n" "\n"
"$Id: cPickleCache.c,v 1.81 2003/04/08 15:55:44 jeremy Exp $\n"; "$Id: cPickleCache.c,v 1.82 2003/04/23 20:05:51 jeremy Exp $\n";
#define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;} #define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;}
#define UNLESS(E) if(!(E)) #define UNLESS(E) if(!(E))
...@@ -104,6 +104,7 @@ static char cPickleCache_doc_string[] = ...@@ -104,6 +104,7 @@ static char cPickleCache_doc_string[] =
#undef Py_FindMethod #undef Py_FindMethod
static PyObject *py__p_oid, *py_reload, *py__p_jar, *py__p_changed; static PyObject *py__p_oid, *py_reload, *py__p_jar, *py__p_changed;
static cPersistenceCAPIstruct *capi;
/* Do we want 'engine noise'.... abstract debugging output useful for /* Do we want 'engine noise'.... abstract debugging output useful for
visualizing cache behavior */ visualizing cache behavior */
...@@ -462,6 +463,46 @@ cc_lru_items(ccobject *self, PyObject *args) ...@@ -462,6 +463,46 @@ cc_lru_items(ccobject *self, PyObject *args)
return l; return l;
} }
/* Be very careful about calling clear().
It removes all non-ghost objects from the ring without otherwise
removing them from the cache. The method should only be called
after the cache is no longer in use.
*/
static PyObject *
cc_clear(ccobject *self, PyObject *args)
{
CPersistentRing *here;
if (!PyArg_ParseTuple(args, ":clear"))
return NULL;
if (self->ring_lock) {
/* When the ring lock is held, we have no way of know which
ring nodes belong to persistent objects, and which a
placeholders. */
PyErr_SetString(PyExc_ValueError,
".lru_items() is unavailable during garbage collection");
return NULL;
}
self->ring_lock = 1;
while ((here = self->ring_home.next) != & self->ring_home) {
cPersistentObject *o = OBJECT_FROM_RING(self, here, "clear");
self->non_ghost_count--;
o->ring.next->prev = &self->ring_home;
self->ring_home.next = o->ring.next;
o->ring.next = NULL;
o->ring.prev = NULL;
Py_DECREF(o);
}
self->ring_lock = 0;
Py_INCREF(Py_None);
return Py_None;
}
static int static int
cc_oid_unreferenced(ccobject *self, PyObject *oid) cc_oid_unreferenced(ccobject *self, PyObject *oid)
{ {
...@@ -570,6 +611,8 @@ static struct PyMethodDef cc_methods[] = { ...@@ -570,6 +611,8 @@ static struct PyMethodDef cc_methods[] = {
"get(key [, default]) -- get an item, or a default"}, "get(key [, default]) -- get an item, or a default"},
{"ringlen", (PyCFunction)cc_ringlen, METH_VARARGS, {"ringlen", (PyCFunction)cc_ringlen, METH_VARARGS,
"ringlen() -- Returns number of non-ghost items in cache."}, "ringlen() -- Returns number of non-ghost items in cache."},
{"clear", (PyCFunction)cc_clear, METH_VARARGS,
"clear() -- remove all objects from the cache"},
{NULL, NULL} /* sentinel */ {NULL, NULL} /* sentinel */
}; };
...@@ -923,7 +966,6 @@ void ...@@ -923,7 +966,6 @@ void
initcPickleCache(void) initcPickleCache(void)
{ {
PyObject *m, *d; PyObject *m, *d;
cPersistenceCAPIstruct *capi;
Cctype.ob_type = &PyType_Type; Cctype.ob_type = &PyType_Type;
......
...@@ -14,7 +14,7 @@ ...@@ -14,7 +14,7 @@
static char cPersistence_doc_string[] = static char cPersistence_doc_string[] =
"Defines Persistent mixin class for persistent objects.\n" "Defines Persistent mixin class for persistent objects.\n"
"\n" "\n"
"$Id: cPersistence.c,v 1.68 2003/04/02 16:50:49 jeremy Exp $\n"; "$Id: cPersistence.c,v 1.69 2003/04/23 20:05:51 jeremy Exp $\n";
#include "cPersistence.h" #include "cPersistence.h"
...@@ -154,7 +154,7 @@ static void ...@@ -154,7 +154,7 @@ static void
accessed(cPersistentObject *self) accessed(cPersistentObject *self)
{ {
/* Do nothing unless the object is in a cache and not a ghost. */ /* Do nothing unless the object is in a cache and not a ghost. */
if (self->cache && self->state >= 0) { if (self->cache && self->state >= 0 && self->ring.next) {
CPersistentRing *home = &self->cache->ring_home; CPersistentRing *home = &self->cache->ring_home;
self->ring.prev->next = self->ring.next; self->ring.prev->next = self->ring.next;
self->ring.next->prev = self->ring.prev; self->ring.next->prev = self->ring.prev;
...@@ -176,6 +176,13 @@ ghostify(cPersistentObject *self) ...@@ -176,6 +176,13 @@ ghostify(cPersistentObject *self)
self->state = cPersistent_GHOST_STATE; self->state = cPersistent_GHOST_STATE;
return; return;
} }
/* If the cache has been cleared, then a non-ghost object
isn't in the ring any longer.
*/
if (self->ring.next == NULL)
return;
/* if we're ghostifying an object, we better have some non-ghosts */ /* if we're ghostifying an object, we better have some non-ghosts */
assert(self->cache->non_ghost_count > 0); assert(self->cache->non_ghost_count > 0);
......
...@@ -90,7 +90,7 @@ process must skip such objects, rather than deactivating them. ...@@ -90,7 +90,7 @@ process must skip such objects, rather than deactivating them.
static char cPickleCache_doc_string[] = static char cPickleCache_doc_string[] =
"Defines the PickleCache used by ZODB Connection objects.\n" "Defines the PickleCache used by ZODB Connection objects.\n"
"\n" "\n"
"$Id: cPickleCache.c,v 1.81 2003/04/08 15:55:44 jeremy Exp $\n"; "$Id: cPickleCache.c,v 1.82 2003/04/23 20:05:51 jeremy Exp $\n";
#define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;} #define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;}
#define UNLESS(E) if(!(E)) #define UNLESS(E) if(!(E))
...@@ -104,6 +104,7 @@ static char cPickleCache_doc_string[] = ...@@ -104,6 +104,7 @@ static char cPickleCache_doc_string[] =
#undef Py_FindMethod #undef Py_FindMethod
static PyObject *py__p_oid, *py_reload, *py__p_jar, *py__p_changed; static PyObject *py__p_oid, *py_reload, *py__p_jar, *py__p_changed;
static cPersistenceCAPIstruct *capi;
/* Do we want 'engine noise'.... abstract debugging output useful for /* Do we want 'engine noise'.... abstract debugging output useful for
visualizing cache behavior */ visualizing cache behavior */
...@@ -462,6 +463,46 @@ cc_lru_items(ccobject *self, PyObject *args) ...@@ -462,6 +463,46 @@ cc_lru_items(ccobject *self, PyObject *args)
return l; return l;
} }
/* Be very careful about calling clear().
It removes all non-ghost objects from the ring without otherwise
removing them from the cache. The method should only be called
after the cache is no longer in use.
*/
static PyObject *
cc_clear(ccobject *self, PyObject *args)
{
CPersistentRing *here;
if (!PyArg_ParseTuple(args, ":clear"))
return NULL;
if (self->ring_lock) {
/* When the ring lock is held, we have no way of know which
ring nodes belong to persistent objects, and which a
placeholders. */
PyErr_SetString(PyExc_ValueError,
".lru_items() is unavailable during garbage collection");
return NULL;
}
self->ring_lock = 1;
while ((here = self->ring_home.next) != & self->ring_home) {
cPersistentObject *o = OBJECT_FROM_RING(self, here, "clear");
self->non_ghost_count--;
o->ring.next->prev = &self->ring_home;
self->ring_home.next = o->ring.next;
o->ring.next = NULL;
o->ring.prev = NULL;
Py_DECREF(o);
}
self->ring_lock = 0;
Py_INCREF(Py_None);
return Py_None;
}
static int static int
cc_oid_unreferenced(ccobject *self, PyObject *oid) cc_oid_unreferenced(ccobject *self, PyObject *oid)
{ {
...@@ -570,6 +611,8 @@ static struct PyMethodDef cc_methods[] = { ...@@ -570,6 +611,8 @@ static struct PyMethodDef cc_methods[] = {
"get(key [, default]) -- get an item, or a default"}, "get(key [, default]) -- get an item, or a default"},
{"ringlen", (PyCFunction)cc_ringlen, METH_VARARGS, {"ringlen", (PyCFunction)cc_ringlen, METH_VARARGS,
"ringlen() -- Returns number of non-ghost items in cache."}, "ringlen() -- Returns number of non-ghost items in cache."},
{"clear", (PyCFunction)cc_clear, METH_VARARGS,
"clear() -- remove all objects from the cache"},
{NULL, NULL} /* sentinel */ {NULL, NULL} /* sentinel */
}; };
...@@ -923,7 +966,6 @@ void ...@@ -923,7 +966,6 @@ void
initcPickleCache(void) initcPickleCache(void)
{ {
PyObject *m, *d; PyObject *m, *d;
cPersistenceCAPIstruct *capi;
Cctype.ob_type = &PyType_Type; Cctype.ob_type = &PyType_Type;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment