Fixed yet another threading bug in handling new oids.

src/CHANGES.txt

@@ -22,6 +22,15 @@ New Features
   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)
 ====================
 

src/ZODB/DemoStorage.py

@@ -58,13 +58,16 @@ class DemoStorage(object):
         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 @@ class DemoStorage(object):
         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 @@ class DemoStorage(object):
 
     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 @@ class DemoStorage(object):
 
     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 @@ class DemoStorage(object):
                 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,

src/ZODB/DemoStorage.test

@@ -365,6 +365,10 @@ DemoStorage keeps up with the issued OIDs to know when not to reissue them...
     >>> 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:
 

src/ZODB/tests/testDemoStorage.py

@@ -128,7 +128,7 @@ def testSomeDelegation():
     ...         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