[Zodb-checkins] SVN: ZODB/branches/3.9/src/ Bug Fixed:

Jim Fulton jim at zope.com
Mon Sep 20 16:08:31 EDT 2010


Log message for revision 116686:
  Bug Fixed:
  - When objects were added in savepoints and either the savepoint was
    rolled back (https://bugs.launchpad.net/zodb/+bug/143560) or the
    transaction was aborted
    (https://mail.zope.org/pipermail/zodb-dev/2010-June/013488.html)
    The objects' _p_oid and _p_jar variables weren't cleared, leading to
    surprizing errors.
  

Changed:
  U   ZODB/branches/3.9/src/CHANGES.txt
  U   ZODB/branches/3.9/src/ZODB/Connection.py
  U   ZODB/branches/3.9/src/ZODB/tests/testConnection.py

-=-
Modified: ZODB/branches/3.9/src/CHANGES.txt
===================================================================
--- ZODB/branches/3.9/src/CHANGES.txt	2010-09-20 19:58:45 UTC (rev 116685)
+++ ZODB/branches/3.9/src/CHANGES.txt	2010-09-20 20:08:30 UTC (rev 116686)
@@ -32,6 +32,13 @@
 
 - Conflict errors didn't invalidate ZEO cache entries.
 
+- When objects were added in savepoints and either the savepoint was
+  rolled back (https://bugs.launchpad.net/zodb/+bug/143560) or the
+  transaction was aborted
+  (https://mail.zope.org/pipermail/zodb-dev/2010-June/013488.html)
+  The objects' _p_oid and _p_jar variables weren't cleared, leading to
+  surprizing errors.
+
 - On Mac OS X, clients that connected and disconnected quickly could
   cause a ZEO server to stop accepting connections, due to a failure
   to catch errors in the initial part of the connection process.

Modified: ZODB/branches/3.9/src/ZODB/Connection.py
===================================================================
--- ZODB/branches/3.9/src/ZODB/Connection.py	2010-09-20 19:58:45 UTC (rev 116685)
+++ ZODB/branches/3.9/src/ZODB/Connection.py	2010-09-20 20:08:30 UTC (rev 116686)
@@ -444,6 +444,8 @@
             assert oid is not None
             if oid in self._added:
                 del self._added[oid]
+                if self._cache.get(oid) is not None:
+                    del self._cache[oid]
                 del obj._p_jar
                 del obj._p_oid
             else:
@@ -749,7 +751,6 @@
         """Disown any objects newly saved in an uncommitted transaction."""
         if creating is None:
             creating = self._creating
-            self._creating = {}
 
         for oid in creating:
             o = self._cache.get(oid)
@@ -758,6 +759,8 @@
                 del o._p_jar
                 del o._p_oid
 
+        creating.clear()
+
     def tpc_vote(self, transaction):
         """Verify that a data manager can commit the transaction."""
         try:
@@ -1130,7 +1133,10 @@
         self._creating.clear()
         self._registered_objects = []
 
-        state = self._storage.position, self._storage.index.copy()
+        state = (self._storage.position,
+                 self._storage.index.copy(),
+                 self._storage.creating.copy(),
+                 )
         result = Savepoint(self, state)
         # While the interface doesn't guarantee this, savepoints are
         # sometimes used just to "break up" very long transactions, and as
@@ -1143,6 +1149,7 @@
         self._abort()
         self._registered_objects = []
         src = self._storage
+        self._invalidate_creating(src.creating)
         index = src.index
         src.reset(*state)
         self._cache.invalidate(index)
@@ -1186,6 +1193,7 @@
     def _abort_savepoint(self):
         """Discard all savepoint data."""
         src = self._savepoint_storage
+        self._invalidate_creating(src.creating)
         self._storage = self._normal_storage
         self._savepoint_storage = None
 
@@ -1205,8 +1213,11 @@
         # by another thread, so the risk of a reread is pretty low.
         # It's really not worth the effort to pursue this.
 
+        # Note that we do this *after* reseting the storage so that, if
+        # data are read, we read it from the reset storage!
+
         self._cache.invalidate(src.index)
-        self._invalidate_creating(src.creating)
+
         src.close()
 
     # Savepoint support
@@ -1334,7 +1345,7 @@
     def temporaryDirectory(self):
         return self._storage.temporaryDirectory()
 
-    def reset(self, position, index):
+    def reset(self, position, index, creating):
         self._file.truncate(position)
         self.position = position
         # Caution:  We're typically called as part of a savepoint rollback.
@@ -1347,6 +1358,7 @@
         # a copy of the index here.  An alternative would be to ensure that
         # all callers pass copies.  As is, our callers do not make copies.
         self.index = index.copy()
+        self.creating = creating
 
 class RootConvenience(object):
 

Modified: ZODB/branches/3.9/src/ZODB/tests/testConnection.py
===================================================================
--- ZODB/branches/3.9/src/ZODB/tests/testConnection.py	2010-09-20 19:58:45 UTC (rev 116685)
+++ ZODB/branches/3.9/src/ZODB/tests/testConnection.py	2010-09-20 20:08:30 UTC (rev 116686)
@@ -556,6 +556,82 @@
     <root: rather_long_name rather_long_name2 rather_long_name4 ...>
     """
 
+class C_invalidations_of_new_objects_work_after_savepoint(Persistent):
+    def __init__(self):
+        self.settings = 1
+
+    def _p_invalidate(self):
+        print 'INVALIDATE', self.settings
+        Persistent._p_invalidate(self)
+        print self.settings   # POSKeyError here
+
+def abort_of_savepoint_creating_new_objects_w_exotic_invalidate_doesnt_break():
+    r"""
+    Before, the following would fail with a POSKeyError, which was
+    somewhat surprizing, in a very edgy sort of way. :)
+
+    Really, when an object add is aborted, the object should be "removed" from
+    the db and its invalidatuon method shouldm't even be called:
+
+    >>> conn = ZODB.connection('data.fs')
+    >>> conn.root.x = x = C_invalidations_of_new_objects_work_after_savepoint()
+    >>> _ = transaction.savepoint()
+    >>> x._p_oid
+    '\x00\x00\x00\x00\x00\x00\x00\x01'
+
+    >>> x._p_jar is conn
+    True
+
+    >>> transaction.abort()
+
+After the abort, the oid and jar are None:
+
+    >>> x._p_oid
+    >>> x._p_jar
+
+Cleanup:
+
+    >>> conn.close()
+
+    """
+
+class Clp9460655(Persistent):
+    def __init__(self, word, id):
+        super(Clp9460655, self).__init__()
+	self.id = id
+        self._word = word
+
+def lp9460655():
+    r"""
+    >>> conn = ZODB.connection('data.fs')
+    >>> root = conn.root()
+    >>> Word = Clp9460655
+
+    >>> from BTrees.OOBTree import OOBTree
+    >>> data = root['data'] = OOBTree()
+
+    >>> commonWords = []
+    >>> count = "0"
+    >>> for x in ('hello', 'world', 'how', 'are', 'you'):
+    ...         commonWords.append(Word(x, count))
+    ...         count = str(int(count) + 1)
+
+    >>> sv = transaction.savepoint()
+    >>> for word in commonWords:
+    ...         sv2 = transaction.savepoint()
+    ...         data[word.id] = word
+
+    >>> sv.rollback()
+    >>> print commonWords[1].id  # raises POSKeyError
+    1
+
+    Cleanup:
+
+    >>> transaction.abort()
+    >>> conn.close()
+
+    """
+
 class _PlayPersistent(Persistent):
     def setValueWithSize(self, size=0): self.value = size*' '
     __init__ = setValueWithSize



More information about the Zodb-checkins mailing list