Commit b4d9b73b 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 d45cea43
......@@ -14,7 +14,7 @@
static char cPersistence_doc_string[] =
"Defines Persistent mixin class for persistent objects.\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"
......@@ -154,7 +154,7 @@ static void
accessed(cPersistentObject *self)
{
/* 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;
self->ring.prev->next = self->ring.next;
self->ring.next->prev = self->ring.prev;
......@@ -176,6 +176,13 @@ ghostify(cPersistentObject *self)
self->state = cPersistent_GHOST_STATE;
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 */
assert(self->cache->non_ghost_count > 0);
......
......@@ -90,7 +90,7 @@ process must skip such objects, rather than deactivating them.
static char cPickleCache_doc_string[] =
"Defines the PickleCache used by ZODB Connection objects.\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 UNLESS(E) if(!(E))
......@@ -104,6 +104,7 @@ static char cPickleCache_doc_string[] =
#undef Py_FindMethod
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
visualizing cache behavior */
......@@ -462,6 +463,46 @@ cc_lru_items(ccobject *self, PyObject *args)
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
cc_oid_unreferenced(ccobject *self, PyObject *oid)
{
......@@ -570,6 +611,8 @@ static struct PyMethodDef cc_methods[] = {
"get(key [, default]) -- get an item, or a default"},
{"ringlen", (PyCFunction)cc_ringlen, METH_VARARGS,
"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 */
};
......@@ -923,7 +966,6 @@ void
initcPickleCache(void)
{
PyObject *m, *d;
cPersistenceCAPIstruct *capi;
Cctype.ob_type = &PyType_Type;
......
......@@ -13,7 +13,7 @@
##############################################################################
"""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
......@@ -131,12 +131,15 @@ class Connection(ExportImport.ExportImport):
return '<Connection at %08x%s>' % (id(self), ver)
def _breakcr(self):
try: del self._cache
except: pass
try: del self._incrgc
except: pass
try: del self.cacheGC
except: pass
# Persistent objects and the cache don't participate in GC.
# Explicitly remove references from the connection to its
# cache and to the root object, because they refer back to the
# connection.
self._cache.clear()
self._cache = None
self._incrgc = None
self.cacheGC = None
self._root_ = None
def __getitem__(self, oid, tt=type(())):
obj = self._cache.get(oid, None)
......
......@@ -13,8 +13,8 @@
##############################################################################
"""Database objects
$Id: DB.py,v 1.49 2003/04/22 18:04:37 jeremy Exp $"""
__version__='$Revision: 1.49 $'[11:-2]
$Id: DB.py,v 1.50 2003/04/23 20:05:51 jeremy Exp $"""
__version__='$Revision: 1.50 $'[11:-2]
import cPickle, cStringIO, sys, POSException, UndoLogCompatible
from Connection import Connection
......@@ -255,6 +255,9 @@ class DB(UndoLogCompatible.UndoLogCompatible):
def close(self):
self._storage.close()
for x, allocated in self._pools[1]:
for c in allocated:
c._breakcr()
def commitVersion(self, source, destination='', transaction=None):
if transaction is None:
......
......@@ -14,7 +14,7 @@
static char cPersistence_doc_string[] =
"Defines Persistent mixin class for persistent objects.\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"
......@@ -154,7 +154,7 @@ static void
accessed(cPersistentObject *self)
{
/* 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;
self->ring.prev->next = self->ring.next;
self->ring.next->prev = self->ring.prev;
......@@ -176,6 +176,13 @@ ghostify(cPersistentObject *self)
self->state = cPersistent_GHOST_STATE;
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 */
assert(self->cache->non_ghost_count > 0);
......
......@@ -90,7 +90,7 @@ process must skip such objects, rather than deactivating them.
static char cPickleCache_doc_string[] =
"Defines the PickleCache used by ZODB Connection objects.\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 UNLESS(E) if(!(E))
......@@ -104,6 +104,7 @@ static char cPickleCache_doc_string[] =
#undef Py_FindMethod
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
visualizing cache behavior */
......@@ -462,6 +463,46 @@ cc_lru_items(ccobject *self, PyObject *args)
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
cc_oid_unreferenced(ccobject *self, PyObject *oid)
{
......@@ -570,6 +611,8 @@ static struct PyMethodDef cc_methods[] = {
"get(key [, default]) -- get an item, or a default"},
{"ringlen", (PyCFunction)cc_ringlen, METH_VARARGS,
"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 */
};
......@@ -923,7 +966,6 @@ void
initcPickleCache(void)
{
PyObject *m, *d;
cPersistenceCAPIstruct *capi;
Cctype.ob_type = &PyType_Type;
......
......@@ -14,7 +14,7 @@
static char cPersistence_doc_string[] =
"Defines Persistent mixin class for persistent objects.\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"
......@@ -154,7 +154,7 @@ static void
accessed(cPersistentObject *self)
{
/* 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;
self->ring.prev->next = self->ring.next;
self->ring.next->prev = self->ring.prev;
......@@ -176,6 +176,13 @@ ghostify(cPersistentObject *self)
self->state = cPersistent_GHOST_STATE;
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 */
assert(self->cache->non_ghost_count > 0);
......
......@@ -90,7 +90,7 @@ process must skip such objects, rather than deactivating them.
static char cPickleCache_doc_string[] =
"Defines the PickleCache used by ZODB Connection objects.\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 UNLESS(E) if(!(E))
......@@ -104,6 +104,7 @@ static char cPickleCache_doc_string[] =
#undef Py_FindMethod
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
visualizing cache behavior */
......@@ -462,6 +463,46 @@ cc_lru_items(ccobject *self, PyObject *args)
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
cc_oid_unreferenced(ccobject *self, PyObject *oid)
{
......@@ -570,6 +611,8 @@ static struct PyMethodDef cc_methods[] = {
"get(key [, default]) -- get an item, or a default"},
{"ringlen", (PyCFunction)cc_ringlen, METH_VARARGS,
"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 */
};
......@@ -923,7 +966,6 @@ void
initcPickleCache(void)
{
PyObject *m, *d;
cPersistenceCAPIstruct *capi;
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