[Zope-Checkins] CVS: Zope/lib/python/ZODB - cPickleCache.c:1.64

[Toby Dickenson]

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

Modified Files:
	cPickleCache.c 
Log Message:
added comments

=== Zope/lib/python/ZODB/cPickleCache.c 1.63 => 1.64 ===
         return -1;
     if (jar==Py_None) {
+        Py_DECREF(jar);
         PyErr_SetString(PyExc_ValueError,
                         "Cached object jar missing");
 	return -1;
@@ -975,11 +976,23 @@
 _ring_corrupt(ccobject *self, const char *context)
 {
     CPersistentRing *here = &(self->ring_home);
+
+    /* Determine the number of objects we expect to see in the ring.
+     * Normally this is one for the home node plus one for each
+     * non-ghost object, for which we maintain a separate total. If the
+     * ring is unlocked then this value should be precise; there should
+     * be no foreign nodes in the ring. If locked, it may be an
+     * underestimate */
     int expected = 1 + self->non_ghost_count;
+
     int total = 0;
     do {
-        if (++total > (expected + 10)) 
-	    return 3;            /* ring too big, by a large margin */
+        if (++total > (expected + 10))
+            /* ring too big, by a large margin. This probably
+             * means we are stomping through random memory. Abort
+             * now, and maybe we can deliver this error message
+             * before dumping core */ 
+	    return 3;            
         if (!here->next)
 	    return 4;                      /* various linking problems */
         if (!here->prev) 
@@ -993,15 +1006,18 @@
         if (here->next->prev != here) 
 	    return 10;
         if (!self->ring_lock) {
-            /* If the ring must be locked, then it only contains
-	       object other than persistent instances.
-	    */ 
+            /* If the ring is unlocked, then it must not contain
+	     * objects other than persistent instances (and the home) */ 
             if (here != &self->ring_home) {
                 cPersistentObject *object = object_from_ring(self, here, 
 							     context);
                 if (!object) 
 		    return 12;
                 if (object->state == cPersistent_GHOST_STATE)
+		    /* ghost objects should not be in the ring, according
+                     * to the ghost storage regime. Experience shows
+                     * that this error condition is likely to be caused
+                     * by a race condition bug somewhere */
                     return 13;
             }
         }
@@ -1009,8 +1025,11 @@
     } while (here != &self->ring_home);
 
     if (self->ring_lock) {
-        if (total < expected) 
-	    return 6;       /* ring too small; too big is ok when locked */
+        if (total < expected)
+            /* ring is too small.
+               too big is ok when locked, we have already checked it is
+               not too big */ 
+	    return 6;       
     } else {
         if (total != expected) 
 	    return 14;     /* ring size wrong, or bad ghost accounting */