[Zodb-checkins] CVS: ZODB3/ZODB - cPickleCache.c:1.68.12.2

[Jeremy Hylton]

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

Modified Files:
      Tag: ZODB3-3_1-branch
	cPickleCache.c 
Log Message:
Backport a variety of small refcount fixes.


=== ZODB3/ZODB/cPickleCache.c 1.68.12.1 => 1.68.12.2 ===
--- ZODB3/ZODB/cPickleCache.c:1.68.12.1	Tue Feb 11 14:15:32 2003
+++ ZODB3/ZODB/cPickleCache.c	Wed Apr 30 11:58:43 2003
@@ -53,10 +53,12 @@
 
 Regime 3: Non-Ghost Objects
 
-Non-ghost objects are stored in two data structures. Firstly, in the
-dictionary along with everything else, with a *strong* reference.
-Secondly, they are stored in a doubly-linked-list which encodes the
-order in which these objects have been most recently used.
+Non-ghost objects are stored in two data structures: the dictionary
+mapping oids to objects and a doubly-linked list that encodes the
+order in which the objects were accessed.  The dictionary reference is
+borrowed, as it is for ghosts.  The list reference is a new reference;
+the list stores recently used objects, even if they are otherwise
+unreferenced, to avoid loading the object from the database again.
 
 The doubly-link-list nodes contain next and previous pointers linking
 together the cache and all non-ghost persistent objects.
@@ -147,18 +149,6 @@
 
 /* ---------------------------------------------------------------- */
 
-/* somewhat of a replacement for PyDict_GetItem(self->data....
-   however this returns a *new* reference */
-static PyObject *
-object_from_oid(ccobject *self, PyObject *key)
-{
-    PyObject *v = PyDict_GetItem(self->data, key);
-    if (!v) 
-	return NULL;
-    Py_INCREF(v);
-    return v;
-}
-
 /* define this for extra debugging checks, and lousy performance.
    Not really necessary in production code... disable this before
    release, providing noone has been reporting and RuntimeErrors
@@ -409,7 +399,7 @@
 static void
 _invalidate(ccobject *self, PyObject *key)
 {
-    PyObject *v = object_from_oid(self, key);
+    PyObject *v = PyDict_GetItem(self->data, key);
 
     if (!v)
 	return;
@@ -430,7 +420,6 @@
 	if (PyObject_DelAttr(v, py__p_changed) < 0)
 	    PyErr_Clear();
     }
-    Py_DECREF(v);
 }
 
 static PyObject *
@@ -480,17 +469,17 @@
     if (!PyArg_ParseTuple(args, "O|O:get", &key, &d)) 
 	return NULL;
 
-    r = (PyObject *)object_from_oid(self, key);
+    r = PyDict_GetItem(self->data, key);
     if (!r) {
 	if (d) {
 	    r = d;
-	    Py_INCREF(r);
 	} else {
 	    PyErr_SetObject(PyExc_KeyError, key);
 	    return NULL;
 	}
     }
 
+    Py_INCREF(r);
     return r;
 }
 
@@ -605,8 +594,11 @@
        interpreter has untracked the reference.  Track it again.
      */
     _Py_NewReference(v);
-    /* XXX it may be a problem that v->ob_type is still NULL? 
-       I don't understand what this comment means.  --jeremy */
+    /* Don't increment total refcount as a result of the 
+       shenanigans played in this function.  The _Py_NewReference()
+       call above creates artificial references to v.
+    */
+    _Py_RefTotal--;
     assert(v->ob_type);
 #else
     Py_INCREF(v);
@@ -774,12 +766,13 @@
     if (IS_RING_CORRUPT(self, "__getitem__")) 
 	return NULL;
 
-    r = (PyObject *)object_from_oid(self, key);
+    r = PyDict_GetItem(self->data, key);
     if (r == NULL) {
 	PyErr_SetObject(PyExc_KeyError, key);
 	return NULL;
     }
 
+    Py_INCREF(r);
     return r;
 }
 
@@ -843,16 +836,14 @@
     }
     Py_DECREF(jar);
 
-    object_again = object_from_oid(self, key);
+    object_again = PyDict_GetItem(self->data, key);
     if (object_again) {
 	if (object_again != v) {
-	    Py_DECREF(object_again);
 	    PyErr_SetString(PyExc_ValueError,
 		    "Can not re-register object under a different oid");
 	    return -1;
 	} else {
 	    /* re-register under the same oid - no work needed */
-	    Py_DECREF(object_again);
 	    return 0;
 	}
     }
@@ -883,22 +874,22 @@
 	return -1;
     if (PyDict_SetItem(self->data, key, v) < 0) 
 	return -1;
+    /* the dict should have a borrowed reference */
+    Py_DECREF(v);
     
     p = (cPersistentObject *)v;
     Py_INCREF(self);
     p->cache = (PerCache *)self;
     if (p->state >= 0) {
 	/* insert this non-ghost object into the ring just 
-	   behind the home position */
+	   behind the home position. */
 	self->non_ghost_count++;
 	p->ring.next = &self->ring_home;
 	p->ring.prev =  self->ring_home.prev;
 	self->ring_home.prev->next = &p->ring;
 	self->ring_home.prev = &p->ring;
-    } else {
-	/* steal a reference from the dictionary; 
-	   ghosts have a weak reference */
-	Py_DECREF(v);
+	/* this list should have a new reference to the object */
+	Py_INCREF(v);
     }
     
     if (IS_RING_CORRUPT(self, "post-setitem")) 
@@ -917,7 +908,7 @@
     if (IS_RING_CORRUPT(self, "pre-delitem")) 
 	return -1;
 
-    v = (PyObject *)object_from_oid(self, key);
+    v = PyDict_GetItem(self->data, key);
     if (v == NULL)
 	return -1;
 
@@ -931,6 +922,8 @@
 	    p->ring.prev->next = p->ring.next;
 	    p->ring.prev = NULL;
 	    p->ring.next = NULL;
+	    /* The DelItem below will account for the reference
+	       held by the list. */
 	} else {
 	    /* This is a ghost object, so we havent kept a reference
 	       count on it.  For it have stayed alive this long
@@ -943,8 +936,6 @@
 	Py_DECREF((PyObject *)p->cache);
 	p->cache = NULL;
     }
-
-    Py_DECREF(v);
 
     if (PyDict_DelItem(self->data, key) < 0) {
 	PyErr_SetString(PyExc_RuntimeError,