[Zope3-checkins] CVS: Zope/lib/python/ZODB - cPickleCache.c:1.68.10.3.4.1

[Toby Dickenson]

Update of /cvs-repository/Zope/lib/python/ZODB
In directory cvs.zope.org:/tmp/cvs-serv13216

Modified Files:
      Tag: toby_terminator_branch
	cPickleCache.c 
Log Message:
Fix for http://zope.org/Collectors/Zope/1208. defend against infinite loops when scanning the cache, by explicitly marking the ring position where we start. There is no possibility of scanning an object more than once, which could previously happen if it had a __del__ method.


=== Zope/lib/python/ZODB/cPickleCache.c 1.68.10.3 => 1.68.10.3.4.1 ===
--- Zope/lib/python/ZODB/cPickleCache.c:1.68.10.3	Wed Apr 30 13:03:34 2003
+++ Zope/lib/python/ZODB/cPickleCache.c	Fri May 14 11:26:53 2004
@@ -155,7 +155,7 @@
    that it uses to report problems.
 */
 
-/* #define MUCH_RING_CHECKING */
+#define MUCH_RING_CHECKING
 
 #ifdef MUCH_RING_CHECKING
 static int present_in_ring(ccobject *self, CPersistentRing *target);
@@ -217,7 +217,7 @@
 
 
 static int
-scan_gc_items(ccobject *self,int target)
+scan_gc_items(ccobject *self,int target,CPersistentRing *terminal)
 {
     /* This function must only be called with the ring lock held,
        because it places a non-object placeholder in the ring.
@@ -266,15 +266,22 @@
         }
 #endif
 
-	/* back to the home position. stop looking */
-        if (here == &self->ring_home)
-            return 0;
+        /* reached the end position. stop looking */
+        if (here == terminal)
+              return 0;
+
+        /* back to the home position. should not happen */
+        if (here == &self->ring_home) {
+            PyErr_SetString(PyExc_RuntimeError,
+                  "reached home before terminal, in scan_gc_items");
+            return -1;
+        }
 
         /* 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 */
+	   from persistent objects, plus our own home node, plus the
+           terminal node. We know this because the ring lock is held.
+           We can safely assume the current ring node is a persistent
+           object now. */
         object = OBJECT_FROM_RING(self, here, "scan_gc_items");
         if (!object) 
 	    return -1;
@@ -326,6 +333,10 @@
 static PyObject *
 lockgc(ccobject *self, int target_size)
 {
+    int error;
+    CPersistentRing terminal;
+
+
     /* This is thread-safe because of the GIL, and there's nothing
      * in between checking the ring_lock and acquiring it that calls back
      * into Python.
@@ -339,11 +350,26 @@
 	return NULL;
     ENGINE_NOISE("<");
     self->ring_lock = 1;
-    if (scan_gc_items(self, target_size)) {
-        self->ring_lock = 0;
-        return NULL;
-    }
+
+    /* insert a placeholder just before the head node. This will be used to
+     * terminate the scan_gc_items loop. Items touched during the
+     * deactivation run will get inserted between the head and this terminal
+     * node, and will not get re-scanned. */
+    terminal.next = &self->ring_home;
+    terminal.prev = self->ring_home.prev;
+    self->ring_home.prev->next = &terminal;
+    self->ring_home.prev = &terminal;
+
+    error = scan_gc_items(self, target_size, &terminal);
+
+    /* unlink the placeholder from the ring */
+    terminal.next->prev = terminal.prev;
+    terminal.prev->next = terminal.next;
+
     self->ring_lock = 0;
+    if (error)
+        return NULL;
+
     ENGINE_NOISE(">\n");
     if (IS_RING_CORRUPT(self, "post-gc")) 
 	return NULL;