[Zodb-checkins] SVN: ZODB/trunk/src/ Fixed yet another threading bug in handling new oids.

[Jim Fulton]

Log message for revision 92817:
  Fixed yet another threading bug in handling new oids.
  

Changed:
  U   ZODB/trunk/src/CHANGES.txt
  U   ZODB/trunk/src/ZODB/DemoStorage.py
  U   ZODB/trunk/src/ZODB/DemoStorage.test
  U   ZODB/trunk/src/ZODB/tests/testDemoStorage.py

-=-
Modified: ZODB/trunk/src/CHANGES.txt
===================================================================
--- ZODB/trunk/src/CHANGES.txt	2008-11-06 17:41:17 UTC (rev 92816)
+++ ZODB/trunk/src/CHANGES.txt	2008-11-06 21:13:41 UTC (rev 92817)
@@ -22,6 +22,15 @@
   XXX There are known issues with this implementation that need to be
   sorted out before it is "released".
 
+3.9.0a4 (2008-11-04)
+====================
+
+Bug Fixes
+---------
+
+- DemoStorage could sometimes hand out the same new object id more
+  than once.
+
 3.9.0a3 (2008-11-04)
 ====================
 

Modified: ZODB/trunk/src/ZODB/DemoStorage.py
===================================================================
--- ZODB/trunk/src/ZODB/DemoStorage.py	2008-11-06 17:41:17 UTC (rev 92816)
+++ ZODB/trunk/src/ZODB/DemoStorage.py	2008-11-06 21:13:41 UTC (rev 92817)
@@ -58,13 +58,16 @@
         self.changes = changes
 
         self._issued_oids = set()
+        self._stored_oids = set()
 
+        self._commit_lock = threading.Lock()
+        self._transaction = None
+
         if name is None:
             name = 'DemoStorage(%r, %r)' % (base.getName(), changes.getName())
         self.__name__ = name
 
         self._copy_methods_from_changes(changes)
-        
 
     def _blobify(self):
         if (self._temporary_changes and
@@ -92,8 +95,7 @@
         for meth in (
             '_lock_acquire', '_lock_release', 
             'getSize', 'history', 'isReadOnly', 'registerDB',
-            'sortKey', 'tpc_begin', 'tpc_abort', 'tpc_finish',
-            'tpc_transaction', 'tpc_vote',
+            'sortKey', 'tpc_transaction', 'tpc_vote',
             ):
             setattr(self, meth, getattr(changes, meth))
 
@@ -229,10 +231,12 @@
 
     def store(self, oid, serial, data, version, transaction):
         assert version=='', "versions aren't supported"
+        if transaction is not self._transaction:
+            raise ZODB.POSException.StorageTransactionError(self, transaction)
 
         # Since the OID is being used, we don't have to keep up with it any
-        # more.
-        self._issued_oids.discard(oid)
+        # more. Save it now so we can forget it later. :)
+        self._stored_oids.add(oid)
 
         # See if we already have changes for this oid
         try:
@@ -251,18 +255,21 @@
 
     def storeBlob(self, oid, oldserial, data, blobfilename, version,
                   transaction):
+        assert version=='', "versions aren't supported"
+        if transaction is not self._transaction:
+            raise ZODB.POSException.StorageTransactionError(self, transaction)
 
         # Since the OID is being used, we don't have to keep up with it any
-        # more.
-        self._issued_oids.discard(oid)
+        # more. Save it now so we can forget it later. :)
+        self._stored_oids.add(oid)
 
         try:
             return self.changes.storeBlob(
-                oid, oldserial, data, blobfilename, version, transaction)
+                oid, oldserial, data, blobfilename, '', transaction)
         except AttributeError:
             if self._blobify():
                 return self.changes.storeBlob(
-                    oid, oldserial, data, blobfilename, version, transaction)
+                    oid, oldserial, data, blobfilename, '', transaction)
             raise
 
     def temporaryDirectory(self):
@@ -273,6 +280,37 @@
                 return self.changes.temporaryDirectory()
             raise
 
+    @ZODB.utils.locked
+    def tpc_abort(self, transaction):
+        if transaction is not self._transaction:
+            return
+        self._stored_oids = set()
+        self._transaction = None
+        self.changes.tpc_abort(transaction)
+        self._commit_lock.release()
+
+    @ZODB.utils.locked
+    def tpc_begin(self, transaction, *a, **k):
+        # The tid argument exists to support testing.
+        if transaction is self._transaction:
+            return
+        self._lock_release()
+        self._commit_lock.acquire()
+        self._lock_acquire()
+        self.changes.tpc_begin(transaction, *a, **k)
+        self._transaction = transaction
+        self._stored_oids = set()
+
+    @ZODB.utils.locked
+    def tpc_finish(self, transaction, func = lambda tid: None):
+        if (transaction is not self._transaction):
+            return
+        self._issued_oids.difference_update(self._stored_oids)
+        self._stored_oids = set()
+        self._transaction = None
+        self.changes.tpc_finish(transaction, func)
+        self._commit_lock.release()
+
 _temporary_blobdirs = {}
 def cleanup_temporary_blobdir(
     ref,

Modified: ZODB/trunk/src/ZODB/DemoStorage.test
===================================================================
--- ZODB/trunk/src/ZODB/DemoStorage.test	2008-11-06 17:41:17 UTC (rev 92816)
+++ ZODB/trunk/src/ZODB/DemoStorage.test	2008-11-06 21:13:41 UTC (rev 92817)
@@ -365,6 +365,10 @@
     >>> t = transaction.begin()
     >>> storage.tpc_begin(t)
     >>> tid = storage.store(oid, 0, 'data', '', t)
+    >>> storage.tpc_vote(t)
+    >>> oid in storage._issued_oids
+    True
+    >>> storage.tpc_finish(t)
 
 ...there's no need to remember it any longer:
 

Modified: ZODB/trunk/src/ZODB/tests/testDemoStorage.py
===================================================================
--- ZODB/trunk/src/ZODB/tests/testDemoStorage.py	2008-11-06 17:41:17 UTC (rev 92816)
+++ ZODB/trunk/src/ZODB/tests/testDemoStorage.py	2008-11-06 21:13:41 UTC (rev 92817)
@@ -128,7 +128,7 @@
     ...         print self.name, 'closed'
     ...     sortKey = getSize = __len__ = history = getTid = None
     ...     tpc_finish = tpc_vote = tpc_transaction = None
-    ...     _lock_acquire = _lock_release = lambda: None
+    ...     _lock_acquire = _lock_release = lambda self: None
     ...     getName = lambda self: 'S'
     ...     isReadOnly = tpc_transaction = None
     ...     supportsUndo = undo = undoLog = undoInfo = None