Revert previous checkin. A few tests failed.

trunk/src/Persistence/cPickleCache.c

@@ -88,7 +88,7 @@ process must skip such objects, rather than deactivating them.
 static char cPickleCache_doc_string[] =
 "Defines the PickleCache used by ZODB Connection objects.\n"
 "\n"
-"$Id: cPickleCache.c,v 1.72 2003/03/31 22:39:33 jeremy Exp $\n";
+"$Id: cPickleCache.c,v 1.73 2003/03/31 22:43:42 jeremy Exp $\n";
 
 #define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;}
 #define UNLESS(E) if(!(E))
@@ -147,6 +147,18 @@ static int cc_ass_sub(ccobject *self, PyObject *key, PyObject *v);
 
 /* ---------------------------------------------------------------- */
 
+/* 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)))
 
@@ -296,7 +308,7 @@ cc_minimize(ccobject *self, PyObject *args)
 static void
 _invalidate(ccobject *self, PyObject *key)
 {
-    PyObject *v = PyDict_GetItem(self->data, key);
+    PyObject *v = object_from_oid(self, key);
 
     if (!v)
 	return;
@@ -317,6 +329,7 @@ _invalidate(ccobject *self, PyObject *key)
 	if (PyObject_DelAttr(v, py__p_changed) < 0)
 	    PyErr_Clear();
     }
+    Py_XDECREF(v);
 }
 
 static PyObject *
@@ -361,21 +374,21 @@ cc_invalidate(ccobject *self, PyObject *args)
 static PyObject *
 cc_get(ccobject *self, PyObject *args)
 {
-    PyObject *r, *key, *def = NULL;
+    PyObject *r, *key, *d = NULL;
 
-    if (!PyArg_ParseTuple(args, "O|O:get", &key, &def)) 
+    if (!PyArg_ParseTuple(args, "O|O:get", &key, &d)) 
 	return NULL;
 
-    r = PyDict_GetItem(self->data, key);
+    r = (PyObject *)object_from_oid(self, key);
     if (!r) {
-	if (def)
-	    r = def;
-	else {
+	if (d) {
+	    r = d;
+	    Py_INCREF(r);
+	} else {
 	    PyErr_SetObject(PyExc_KeyError, key);
 	    return NULL;
 	}
     }
-    Py_INCREF(r);
 
     return r;
 }
@@ -488,13 +501,6 @@ cc_oid_unreferenced(ccobject *self, PyObject *oid)
        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);
@@ -511,7 +517,6 @@ cc_oid_unreferenced(ccobject *self, PyObject *oid)
     /* 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,
@@ -659,12 +664,11 @@ cc_subscript(ccobject *self, PyObject *key)
 {
     PyObject *r;
 
-    r = PyDict_GetItem(self->data, key);
+    r = (PyObject *)object_from_oid(self, key);
     if (r == NULL) {
 	PyErr_SetObject(PyExc_KeyError, key);
 	return NULL;
     }
-    Py_INCREF(r);
 
     return r;
 }
@@ -682,8 +686,8 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
     else if( PyExtensionInstance_Check(v) &&
              (((PyExtensionClass*)(v->ob_type))->class_flags & PERSISTENT_TYPE_FLAG) &&
              (v->ob_type->tp_basicsize >= sizeof(cPersistentObject)) ) {
-        /* Its an instance of a persistent class, (ie Python classes that
-	   derive from Persistence.Persistent, BTrees, etc). Thats ok. */
+        /* Its and instance of a persistent class, (ie Python classeses that
+        derive from Persistence.Persistent, BTrees, etc). Thats ok. */
     }
     else {
 	PyErr_SetString(PyExc_TypeError, 
@@ -721,7 +725,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
     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");
@@ -729,14 +733,16 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
     }
     Py_DECREF(jar);
 
-    object_again = PyDict_GetItem(self->data, key);
+    object_again = object_from_oid(self, 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;
 	}
     }
@@ -793,7 +799,7 @@ cc_del_item(ccobject *self, PyObject *key)
     cPersistentObject *p;
 
     /* unlink this item from the ring */
-    v = PyDict_GetItem(self->data, key);
+    v = (PyObject *)object_from_oid(self, key);
     if (v == NULL)
 	return -1;
 
@@ -820,6 +826,8 @@ cc_del_item(ccobject *self, PyObject *key)
 	p->cache = NULL;
     }
 
+    Py_DECREF(v);
+
     if (PyDict_DelItem(self->data, key) < 0) {
 	PyErr_SetString(PyExc_RuntimeError,
 			"unexpectedly couldn't remove key in cc_ass_sub");

trunk/src/ZODB/cPickleCache.c

@@ -88,7 +88,7 @@ process must skip such objects, rather than deactivating them.
 static char cPickleCache_doc_string[] =
 "Defines the PickleCache used by ZODB Connection objects.\n"
 "\n"
-"$Id: cPickleCache.c,v 1.72 2003/03/31 22:39:33 jeremy Exp $\n";
+"$Id: cPickleCache.c,v 1.73 2003/03/31 22:43:42 jeremy Exp $\n";
 
 #define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;}
 #define UNLESS(E) if(!(E))
@@ -147,6 +147,18 @@ static int cc_ass_sub(ccobject *self, PyObject *key, PyObject *v);
 
 /* ---------------------------------------------------------------- */
 
+/* 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)))
 
@@ -296,7 +308,7 @@ cc_minimize(ccobject *self, PyObject *args)
 static void
 _invalidate(ccobject *self, PyObject *key)
 {
-    PyObject *v = PyDict_GetItem(self->data, key);
+    PyObject *v = object_from_oid(self, key);
 
     if (!v)
 	return;
@@ -317,6 +329,7 @@ _invalidate(ccobject *self, PyObject *key)
 	if (PyObject_DelAttr(v, py__p_changed) < 0)
 	    PyErr_Clear();
     }
+    Py_XDECREF(v);
 }
 
 static PyObject *
@@ -361,21 +374,21 @@ cc_invalidate(ccobject *self, PyObject *args)
 static PyObject *
 cc_get(ccobject *self, PyObject *args)
 {
-    PyObject *r, *key, *def = NULL;
+    PyObject *r, *key, *d = NULL;
 
-    if (!PyArg_ParseTuple(args, "O|O:get", &key, &def)) 
+    if (!PyArg_ParseTuple(args, "O|O:get", &key, &d)) 
 	return NULL;
 
-    r = PyDict_GetItem(self->data, key);
+    r = (PyObject *)object_from_oid(self, key);
     if (!r) {
-	if (def)
-	    r = def;
-	else {
+	if (d) {
+	    r = d;
+	    Py_INCREF(r);
+	} else {
 	    PyErr_SetObject(PyExc_KeyError, key);
 	    return NULL;
 	}
     }
-    Py_INCREF(r);
 
     return r;
 }
@@ -488,13 +501,6 @@ cc_oid_unreferenced(ccobject *self, PyObject *oid)
        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);
@@ -511,7 +517,6 @@ cc_oid_unreferenced(ccobject *self, PyObject *oid)
     /* 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,
@@ -659,12 +664,11 @@ cc_subscript(ccobject *self, PyObject *key)
 {
     PyObject *r;
 
-    r = PyDict_GetItem(self->data, key);
+    r = (PyObject *)object_from_oid(self, key);
     if (r == NULL) {
 	PyErr_SetObject(PyExc_KeyError, key);
 	return NULL;
     }
-    Py_INCREF(r);
 
     return r;
 }
@@ -682,8 +686,8 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
     else if( PyExtensionInstance_Check(v) &&
              (((PyExtensionClass*)(v->ob_type))->class_flags & PERSISTENT_TYPE_FLAG) &&
              (v->ob_type->tp_basicsize >= sizeof(cPersistentObject)) ) {
-        /* Its an instance of a persistent class, (ie Python classes that
-	   derive from Persistence.Persistent, BTrees, etc). Thats ok. */
+        /* Its and instance of a persistent class, (ie Python classeses that
+        derive from Persistence.Persistent, BTrees, etc). Thats ok. */
     }
     else {
 	PyErr_SetString(PyExc_TypeError, 
@@ -721,7 +725,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
     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");
@@ -729,14 +733,16 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
     }
     Py_DECREF(jar);
 
-    object_again = PyDict_GetItem(self->data, key);
+    object_again = object_from_oid(self, 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;
 	}
     }
@@ -793,7 +799,7 @@ cc_del_item(ccobject *self, PyObject *key)
     cPersistentObject *p;
 
     /* unlink this item from the ring */
-    v = PyDict_GetItem(self->data, key);
+    v = (PyObject *)object_from_oid(self, key);
     if (v == NULL)
 	return -1;
 
@@ -820,6 +826,8 @@ cc_del_item(ccobject *self, PyObject *key)
 	p->cache = NULL;
     }
 
+    Py_DECREF(v);
+
     if (PyDict_DelItem(self->data, key) < 0) {
 	PyErr_SetString(PyExc_RuntimeError,
 			"unexpectedly couldn't remove key in cc_ass_sub");

trunk/src/persistent/cPickleCache.c

@@ -88,7 +88,7 @@ process must skip such objects, rather than deactivating them.
 static char cPickleCache_doc_string[] =
 "Defines the PickleCache used by ZODB Connection objects.\n"
 "\n"
-"$Id: cPickleCache.c,v 1.72 2003/03/31 22:39:33 jeremy Exp $\n";
+"$Id: cPickleCache.c,v 1.73 2003/03/31 22:43:42 jeremy Exp $\n";
 
 #define ASSIGN(V,E) {PyObject *__e; __e=(E); Py_XDECREF(V); (V)=__e;}
 #define UNLESS(E) if(!(E))
@@ -147,6 +147,18 @@ static int cc_ass_sub(ccobject *self, PyObject *key, PyObject *v);
 
 /* ---------------------------------------------------------------- */
 
+/* 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)))
 
@@ -296,7 +308,7 @@ cc_minimize(ccobject *self, PyObject *args)
 static void
 _invalidate(ccobject *self, PyObject *key)
 {
-    PyObject *v = PyDict_GetItem(self->data, key);
+    PyObject *v = object_from_oid(self, key);
 
     if (!v)
 	return;
@@ -317,6 +329,7 @@ _invalidate(ccobject *self, PyObject *key)
 	if (PyObject_DelAttr(v, py__p_changed) < 0)
 	    PyErr_Clear();
     }
+    Py_XDECREF(v);
 }
 
 static PyObject *
@@ -361,21 +374,21 @@ cc_invalidate(ccobject *self, PyObject *args)
 static PyObject *
 cc_get(ccobject *self, PyObject *args)
 {
-    PyObject *r, *key, *def = NULL;
+    PyObject *r, *key, *d = NULL;
 
-    if (!PyArg_ParseTuple(args, "O|O:get", &key, &def)) 
+    if (!PyArg_ParseTuple(args, "O|O:get", &key, &d)) 
 	return NULL;
 
-    r = PyDict_GetItem(self->data, key);
+    r = (PyObject *)object_from_oid(self, key);
     if (!r) {
-	if (def)
-	    r = def;
-	else {
+	if (d) {
+	    r = d;
+	    Py_INCREF(r);
+	} else {
 	    PyErr_SetObject(PyExc_KeyError, key);
 	    return NULL;
 	}
     }
-    Py_INCREF(r);
 
     return r;
 }
@@ -488,13 +501,6 @@ cc_oid_unreferenced(ccobject *self, PyObject *oid)
        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);
@@ -511,7 +517,6 @@ cc_oid_unreferenced(ccobject *self, PyObject *oid)
     /* 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,
@@ -659,12 +664,11 @@ cc_subscript(ccobject *self, PyObject *key)
 {
     PyObject *r;
 
-    r = PyDict_GetItem(self->data, key);
+    r = (PyObject *)object_from_oid(self, key);
     if (r == NULL) {
 	PyErr_SetObject(PyExc_KeyError, key);
 	return NULL;
     }
-    Py_INCREF(r);
 
     return r;
 }
@@ -682,8 +686,8 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
     else if( PyExtensionInstance_Check(v) &&
              (((PyExtensionClass*)(v->ob_type))->class_flags & PERSISTENT_TYPE_FLAG) &&
              (v->ob_type->tp_basicsize >= sizeof(cPersistentObject)) ) {
-        /* Its an instance of a persistent class, (ie Python classes that
-	   derive from Persistence.Persistent, BTrees, etc). Thats ok. */
+        /* Its and instance of a persistent class, (ie Python classeses that
+        derive from Persistence.Persistent, BTrees, etc). Thats ok. */
     }
     else {
 	PyErr_SetString(PyExc_TypeError, 
@@ -721,7 +725,7 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
     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");
@@ -729,14 +733,16 @@ cc_add_item(ccobject *self, PyObject *key, PyObject *v)
     }
     Py_DECREF(jar);
 
-    object_again = PyDict_GetItem(self->data, key);
+    object_again = object_from_oid(self, 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;
 	}
     }
@@ -793,7 +799,7 @@ cc_del_item(ccobject *self, PyObject *key)
     cPersistentObject *p;
 
     /* unlink this item from the ring */
-    v = PyDict_GetItem(self->data, key);
+    v = (PyObject *)object_from_oid(self, key);
     if (v == NULL)
 	return -1;
 
@@ -820,6 +826,8 @@ cc_del_item(ccobject *self, PyObject *key)
 	p->cache = NULL;
     }
 
+    Py_DECREF(v);
+
     if (PyDict_DelItem(self->data, key) < 0) {
 	PyErr_SetString(PyExc_RuntimeError,
 			"unexpectedly couldn't remove key in cc_ass_sub");