[Zope-Checkins] CVS: ZODB3/ZODB - cPickleCache.c:1.72

[Jeremy Hylton]

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

Modified Files:
	cPickleCache.c 
Log Message:
Fix several ref count bugs.

Remove object_from_oid() which appeared to be mis-used about half the
time.  Instead use std PyDict_GetItem() and incref only when
necessary.

Make sure an object's reference to its cache is decref'd when the
object is deallocated.  This change seems to eliminate leaking pickle
caches.

Remaining mystery: Why do objects stay in the cache even when there
are no references to them?


=== ZODB3/ZODB/cPickleCache.c 1.71 => 1.72 ===
--- ZODB3/ZODB/cPickleCache.c:1.71	Tue Feb 11 14:16:27 2003
+++ ZODB3/ZODB/cPickleCache.c	Mon Mar 31 17:39:33 2003
@@ -147,18 +147,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 OBJECT_FROM_RING(SELF, HERE, CTX) \
     ((cPersistentObject *)(((char *)here) - offsetof(cPersistentObject, ring)))
 
@@ -308,7 +296,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;
@@ -329,7 +317,6 @@
 	if (PyObject_DelAttr(v, py__p_changed) < 0)
 	    PyErr_Clear();
     }
-    Py_XDECREF(v);
 }
 
 static PyObject *
@@ -374,21 +361,21 @@
 static PyObject *
 cc_get(ccobject *self, PyObject *args)
 {
-    PyObject *r, *key, *d = NULL;
+    PyObject *r, *key, *def = NULL;
 
-    if (!PyArg_ParseTuple(args, "O|O:get", &key, &d)) 
+    if (!PyArg_ParseTuple(args, "O|O:get", &key, &def)) 
 	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 {
+	if (def)
+	    r = def;
+	else {
 	    PyErr_SetObject(PyExc_KeyError, key);
 	    return NULL;
 	}
     }
+    Py_INCREF(r);
 
     return r;
 }
@@ -501,6 +488,13 @@
        interpreter has untracked the reference.  Track it again.
      */
     _Py_NewReference(v);
+
+    /* Don't increment total refcount as a result of the 
+       shenanigans played in this function.  The _Py_NewReference()
+       call above and the Py_INCREF() below both create artificial
+       references to v.
+    */
+    _Py_RefTotal -= 2;
     /* XXX it may be a problem that v->ob_type is still NULL? 
        I don't understand what this comment means.  --jeremy */
     assert(v->ob_type);
@@ -517,6 +511,7 @@
     /* XXX Should we call _Py_ForgetReference() on error exit? */
     if (PyDict_DelItem(self->data, oid) < 0)
 	return -1;
+    Py_DECREF((ccobject *)((cPersistentObject *)v)->cache);
 
     if (v->ob_refcnt != 1) {
         PyErr_SetString(PyExc_ValueError,
@@ -664,11 +659,12 @@
 {
     PyObject *r;
 
-    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;
 }
@@ -686,8 +682,8 @@
     else if( PyExtensionInstance_Check(v) &&
              (((PyExtensionClass*)(v->ob_type))->class_flags & PERSISTENT_TYPE_FLAG) &&
              (v->ob_type->tp_basicsize >= sizeof(cPersistentObject)) ) {
-        /* Its and instance of a persistent class, (ie Python classeses that
-        derive from Persistence.Persistent, BTrees, etc). Thats ok. */
+        /* Its an instance of a persistent class, (ie Python classes that
+	   derive from Persistence.Persistent, BTrees, etc). Thats ok. */
     }
     else {
 	PyErr_SetString(PyExc_TypeError, 
@@ -725,7 +721,7 @@
     jar = PyObject_GetAttr(v, py__p_jar);
     if (jar == NULL)
         return -1;
-    if (jar==Py_None) {
+    if (jar == Py_None) {
         Py_DECREF(jar);
         PyErr_SetString(PyExc_ValueError,
                         "Cached object jar missing");
@@ -733,16 +729,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;
 	}
     }
@@ -799,7 +793,7 @@
     cPersistentObject *p;
 
     /* unlink this item from the ring */
-    v = (PyObject *)object_from_oid(self, key);
+    v = PyDict_GetItem(self->data, key);
     if (v == NULL)
 	return -1;
 
@@ -825,8 +819,6 @@
 	Py_DECREF((PyObject *)p->cache);
 	p->cache = NULL;
     }
-
-    Py_DECREF(v);
 
     if (PyDict_DelItem(self->data, key) < 0) {
 	PyErr_SetString(PyExc_RuntimeError,