[Zope3-checkins] CVS: Zope3/src/zodb - connection.py:1.39

[Marius Gedminas]

Update of /cvs-repository/Zope3/src/zodb
In directory cvs.zope.org:/tmp/cvs-serv27246

Modified Files:
	connection.py 
Log Message:
There was a problem with modifying a persistent object during its
__getstate__ method, during a transaction savepoint or commit.
There was also a similar problem when calling connection.add() during
the time a connection is preparing to commit.
These are all different manifestations of the same problem: the registered
objects mapping in a connection cannot be modified during an iteration over
its keys or values or items.

We have solved this problem by using a data structure for the registered
object mapping that maintains a list of new keys as they are added to the
mapping. A client of the mapping can take a copy of the keys or values,
with _registered.values(), iterate over that to do work, and then iterate
using _registered.iterAddedKeys() to do work on objects added to the
mapping during the original iteration, and during the current iteration.

If the code in the Connection class ever removed items from the registered
objects mapping then it would need to check that the key from iterAddedKeys()
is still in the registered objects mapping when iterating over it.

However, the code in the Connection class never removes items from the
registered objects mapping. The mapping is cleared in commit and abort.
However, commit and abort cannot be called during prepare or savepoint, as
the transaction is in the wrong state. So, we can safely assume that the
keys listed by iterAddedKeys() are actually in the mapping.

The problem originally described does not occur in ZODB 3. We expect that
this is because of the different data structures, and ways of processing
them used in ZODB 3's Connection code.

(Steve originally wanted to commit with just the text "F*xed B*g", but
Marius convinced him to provide more explanation.)



=== Zope3/src/zodb/connection.py 1.38 => 1.39 ===
--- Zope3/src/zodb/connection.py:1.38	Thu Oct  9 14:04:58 2003
+++ Zope3/src/zodb/connection.py	Wed Oct 15 08:00:19 2003
@@ -59,6 +59,39 @@
 from persistence.cache import Cache
 from persistence.interfaces import IPersistentDataManager
 
+
+class RegisteredMapping(dict):
+    """Mapping used for Connection._registered.
+
+    This mapping must support additions and clears during iteration over
+    values.
+    """
+
+    def __init__(self, *args, **kw):
+        dict.__init__(self, *args, **kw)
+        self._added_keys = []
+
+    def __setitem__(self, key, value):
+        if key not in self:
+             self._added_keys.append(key)
+        dict.__setitem__(self, key, value)
+
+    def setdefault(self, key, value=None):
+        if key not in self:
+             self._added_keys.append(key)
+        dict.setdefault(self, key, value)
+
+    def update(self, other):
+        self._added_keys.extend([key for key in other if key not in self])
+        dict.update(self, other)
+
+    def iterAddedKeys(self):
+        return iter(self._added_keys)
+
+    def clearAddedKeys(self):
+        del self._added_keys[:]
+
+
 class Connection(ExportImport, object):
     """Object managers for individual object space.
 
@@ -107,7 +140,7 @@
         # These sets are clear()ed at transaction boundaries.
 
         # XXX Is a Set safe?  What if the objects are not hashable?
-        self._registered = {}
+        self._registered = RegisteredMapping()
         self._modified = Set() # XXX is this the same as registered?
         self._created = Set()
         # _conflicts: set of objects that failed to load because
@@ -305,8 +338,8 @@
         if obj._p_jar is None:
             # Setting _p_changed has a side-effect of adding obj to
             # _p_jar._registered, so it must be set after _p_jar.
-            obj._p_jar = self
             obj._p_oid = self.newObjectId()
+            obj._p_jar = self
             obj._p_changed = True
             self._created.add(obj._p_oid)
 
@@ -337,7 +370,13 @@
         else:
             self._storage.tpcBegin(txn)
 
-        for obj in self._registered.itervalues():
+        self._registered.clearAddedKeys()
+        for obj in self._registered.values():
+            self._objcommit(obj, txn)
+        for oid in self._registered.iterAddedKeys():
+            # _registered can have new items added to it during _objcommit,
+            # but it cannot have any existing ones removed
+            obj = self._registered[oid]
             self._objcommit(obj, txn)
 
         s = self._storage.tpcVote(txn)
@@ -387,7 +426,13 @@
         self._created = Set()
         self._storage.tpcBegin(txn)
 
-        for obj in self._registered.itervalues():
+        self._registered.clearAddedKeys()
+        for obj in self._registered.values():
+            self._objcommit(obj, txn)
+        for oid in self._registered.iterAddedKeys():
+            # _registered can have new items added to it during _objcommit,
+            # but it cannot have any existing ones removed
+            obj = self._registered[oid]
             self._objcommit(obj, txn)
         self.importHook(txn) # hook for ExportImport