[Zodb-checkins] CVS: ZODB3/ZODB - Connection.py:1.90 DB.py:1.50 cPersistence.c:1.69 cPickleCache.c:1.82

[Jeremy Hylton]

Update of /cvs-repository/ZODB3/ZODB
In directory cvs.zope.org:/tmp/cvs-serv10088/ZODB

Modified Files:
	Connection.py DB.py cPersistence.c cPickleCache.c 
Log Message:
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.


=== ZODB3/ZODB/Connection.py 1.89 => 1.90 ===
--- ZODB3/ZODB/Connection.py:1.89	Tue Apr 22 14:04:37 2003
+++ ZODB3/ZODB/Connection.py	Wed Apr 23 16:05:51 2003
@@ -131,12 +131,15 @@
         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)


=== ZODB3/ZODB/DB.py 1.49 => 1.50 ===
--- ZODB3/ZODB/DB.py:1.49	Tue Apr 22 14:04:37 2003
+++ ZODB3/ZODB/DB.py	Wed Apr 23 16:05:51 2003
@@ -255,6 +255,9 @@
 
     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:


=== ZODB3/ZODB/cPersistence.c 1.68 => 1.69 ===
--- ZODB3/ZODB/cPersistence.c:1.68	Wed Apr  2 11:50:49 2003
+++ ZODB3/ZODB/cPersistence.c	Wed Apr 23 16:05:51 2003
@@ -154,7 +154,7 @@
 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 @@
         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);
 


=== ZODB3/ZODB/cPickleCache.c 1.81 => 1.82 ===
--- ZODB3/ZODB/cPickleCache.c:1.81	Tue Apr  8 11:55:44 2003
+++ ZODB3/ZODB/cPickleCache.c	Wed Apr 23 16:05:51 2003
@@ -104,6 +104,7 @@
 #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 @@
     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 @@
    "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 @@
 initcPickleCache(void)
 {
     PyObject *m, *d;
-    cPersistenceCAPIstruct *capi;
 
     Cctype.ob_type = &PyType_Type;