[Zope-Checkins] CVS: Zope/lib/python/ZODB - DemoStorage.py:1.14.2.1 Transaction.py:1.37.6.4 cPickleCache.c:1.68.10.3

Jeremy Hylton jeremy@zope.com
Wed, 30 Apr 2003 13:03:35 -0400


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

Modified Files:
      Tag: Zope-2_6-branch
	DemoStorage.py Transaction.py cPickleCache.c 
Log Message:
Backport bug fixes.

DemoStorage:
Fix undoLog() to process arguments correctly.
Fix edge case in pack() -- backpointer to object created in version.

Transaction:
If an error occurs during tpc_finish, save the original exception just
in case something goes wrong while cleaning up the message.

cPickleCache: 
A variety of small refcount fixes.


=== Zope/lib/python/ZODB/DemoStorage.py 1.14 => 1.14.2.1 ===
--- Zope/lib/python/ZODB/DemoStorage.py:1.14	Wed Aug 28 14:48:59 2002
+++ Zope/lib/python/ZODB/DemoStorage.py	Wed Apr 30 13:03:34 2003
@@ -90,7 +90,6 @@
 class DemoStorage(BaseStorage.BaseStorage):
 
     def __init__(self, name='Demo Storage', base=None, quota=None):
-
         BaseStorage.BaseStorage.__init__(self, name, base)
 
         # We use a BTree because the items are sorted!
@@ -141,8 +140,8 @@
             oids = []
             for r in v.values():
                 oid, serial, pre, (version, nv), p = r
+                oids.append(oid)
                 if nv:
-                    oids.append(oid)
                     oid, serial, pre, vdata, p = nv
                     self._tindex.append([oid, serial, r, None, p])
                 else:
@@ -226,13 +225,16 @@
 
         self._lock_acquire()
         try:
-            old=self._index.get(oid, None)
+            old = self._index.get(oid, None)
             if old is None:
                 # Hm, nothing here, check the base version:
-                try: p, oserial = self._base.load(oid, '')
-                except: pass
-                else:
-                    old= oid, oserial, None, None, p
+                if self._base:
+                    try:
+                        p, oserial = self._base.load(oid, '')
+                    except KeyError:
+                        pass
+                    else:
+                        old = oid, oserial, None, None, p
 
             nv=None
             if old:
@@ -355,10 +357,11 @@
         finally: self._lock_release()
 
     def undoLog(self, first, last, filter=None):
-        # I think this is wrong given the handling of first and last
-        # in FileStorage.
+        if last < 0:
+            last = first - last + 1
         self._lock_acquire()
         try:
+            # XXX Shouldn't this be sorted?
             transactions = self._data.items()
             pos = len(transactions)
             r = []
@@ -452,9 +455,9 @@
                 # Scan non-version pickle for references
                 r=index.get(oid, None)
                 if r is None:
-                    # Base storage
-                    p, s = self._base.load(oid, '')
-                    referencesf(p, rootl)
+                    if self._base:
+                        p, s = self._base.load(oid, '')
+                        referencesf(p, rootl)
                 else:
                     pindex[oid]=r
                     oid, serial, pre, vdata, p = r
@@ -504,25 +507,27 @@
 
                 if o:
                     if len(o) != len(t):
-                        _data[tid]=1, u, d, e, tuple(o) # Reset data
+                        _data[tid] = 1, u, d, e, tuple(o) # Reset data
                 else:
                     deleted.append(tid)
 
             # Now delete empty transactions
-            for tid in deleted: del _data[tid]
+            for tid in deleted:
+                del _data[tid]
 
             # Now reset previous pointers for "current" records:
             for r in pindex.values():
-                r[2]=None # Previous record
-                if r[3]: # vdata
-                    r[3][1][2]=None
-
-            pindex=None
+                r[2] = None # Previous record
+                if r[3] and r[3][1]: # vdata
+                    # If this record contains version data and
+                    # non-version data, then clear it out.
+                    r[3][1][2] = None
 
             # Finally, rebuild indexes from transaction data:
             self._index, self._vindex = self._build_indexes()
 
-        finally: self._lock_release()
+        finally:
+            self._lock_release()
         self.getSize()
 
     def _splat(self):


=== Zope/lib/python/ZODB/Transaction.py 1.37.6.3 => 1.37.6.4 ===
--- Zope/lib/python/ZODB/Transaction.py:1.37.6.3	Thu Jan 30 18:25:13 2003
+++ Zope/lib/python/ZODB/Transaction.py	Wed Apr 30 13:03:34 2003
@@ -257,6 +257,7 @@
                 # have to clean up.  First save the original exception
                 # in case the cleanup process causes another
                 # exception.
+                error = sys.exc_info()
                 try:
                     self._commit_error(objects, ncommitted, jars, subjars)
                 except:
@@ -264,7 +265,7 @@
                         "A storage error occured during transaction "
                         "abort.  This shouldn't happen.",
                         error=sys.exc_info())
-                raise
+                raise error[0], error[1], error[2]
         finally:
             del objects[:] # clear registered
             if not subtransaction and self._id is not None:


=== Zope/lib/python/ZODB/cPickleCache.c 1.68.10.2 => 1.68.10.3 ===
--- Zope/lib/python/ZODB/cPickleCache.c:1.68.10.2	Thu Jan 30 18:25:13 2003
+++ Zope/lib/python/ZODB/cPickleCache.c	Wed Apr 30 13:03:34 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 *
@@ -439,11 +428,6 @@
   PyObject *inv, *key, *v;
   int i = 0;
 
-  /* Older versions of ZODB used None to mean "invalidate everything,"
-     but current Connection implementations don't ever pass None.
-  */
-  assert(key != Py_None);
-
   if (PyArg_ParseTuple(args, "O!", &PyDict_Type, &inv)) {
       while (PyDict_Next(inv, &i, &key, &v))
 	  _invalidate(self, key);
@@ -485,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;
 }
 
@@ -610,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);
@@ -779,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;
 }
 
@@ -848,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;
 	}
     }
@@ -888,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")) 
@@ -922,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;
 
@@ -936,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
@@ -948,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,