Don't use self._transaction.

It's not thread-safe to store a Transaction() object in an instance
variable without some sort of locking.  Nor is it desirable to use the
same transaction object for more than one transaction.

So get rid of all uses of self._transaction and replace with a local
variable.

src/ZODB/tests/BasicStorage.py

@@ -19,14 +19,15 @@ ZERO = '\0'*8
 
 class BasicStorage:
     def checkBasics(self):
-        self._storage.tpc_begin(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
         # This should simply return
-        self._storage.tpc_begin(self._transaction)
+        self._storage.tpc_begin(t)
         # Aborting is easy
-        self._storage.tpc_abort(self._transaction)
+        self._storage.tpc_abort(t)
         # Test a few expected exceptions when we're doing operations giving a
         # different Transaction object than the one we've begun on.
-        self._storage.tpc_begin(self._transaction)
+        self._storage.tpc_begin(t)
         self.assertRaises(
             POSException.StorageTransactionError,
             self._storage.store,
@@ -65,7 +66,7 @@ class BasicStorage:
             POSException.StorageTransactionError,
             self._storage.store,
             0, 1, 2, 3, Transaction())
-        self._storage.tpc_abort(self._transaction)
+        self._storage.tpc_abort(t)
 
     def checkSerialIsNoneForInitialRevision(self):
         eq = self.assertEqual
@@ -125,7 +126,6 @@ class BasicStorage:
         # Now abort this transaction
         self._storage.tpc_abort(self._transaction)
         # Now start all over again
-        self._transaction = Transaction()
         oid = self._storage.new_oid()
         self._dostore(oid=oid, data=MinPO(6))
 
@@ -133,14 +133,13 @@ class BasicStorage:
         oid1 = self._storage.new_oid()
         revid1 = self._dostore(oid=oid1, data=MinPO(-2))
         oid = self._storage.new_oid()
-        self._storage.tpc_begin(self._transaction)
-        self._storage.store(oid, ZERO, zodb_pickle(MinPO(5)),
-                            '', self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        self._storage.store(oid, ZERO, zodb_pickle(MinPO(5)), '', t)
         # Now abort this transaction
-        self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_abort(self._transaction)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_abort(t)
         # Now start all over again
-        self._transaction = Transaction()
         oid = self._storage.new_oid()
         revid = self._dostore(oid=oid, data=MinPO(6))
 

src/ZODB/tests/ConflictResolution.py

@@ -144,9 +144,10 @@ class ConflictResolvingTransUndoStorage:
         # Start the undo
         info = self._storage.undoInfo()
         tid = info[1]['id']
-        self._storage.tpc_begin(self._transaction)
-        self._storage.transactionalUndo(tid, self._transaction)
-        self._storage.tpc_finish(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        self._storage.transactionalUndo(tid, t)
+        self._storage.tpc_finish(t)
 
     def checkUndoUnresolvable(self):
         # This test is based on checkNotUndoable in the
@@ -164,9 +165,9 @@ class ConflictResolvingTransUndoStorage:
         # Start the undo
         info = self._storage.undoInfo()
         tid = info[1]['id']
-        self._storage.tpc_begin(self._transaction)
-        self.assertRaises(UndoError,
-                          self._storage.transactionalUndo,
-                          tid, self._transaction)
-        self._storage.tpc_abort(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        self.assertRaises(UndoError, self._storage.transactionalUndo,
+                          tid, t)
+        self._storage.tpc_abort(t)
 

src/ZODB/tests/HistoryStorage.py

@@ -4,6 +4,7 @@ Any storage that supports the history() method should be able to pass
 all these tests.
 """
 
+from ZODB.Transaction import Transaction
 from ZODB.tests.MinPO import MinPO
 from ZODB.tests.StorageTestBase import zodb_unpickle
 
@@ -111,10 +112,11 @@ class HistoryStorage:
         revid6 = self._dostore(oid, revid=revid5, data=MinPO(16),
                                version=version)
         # Now commit the version
-        self._storage.tpc_begin(self._transaction)
-        oids = self._storage.commitVersion(version, '', self._transaction)
-        self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_finish(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        oids = self._storage.commitVersion(version, '', t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
         # After consultation with Jim, we agreed that the semantics of
         # revision id's after a version commit is that the committed object
         # gets a new serial number (a.k.a. revision id).  Note that
@@ -168,10 +170,11 @@ class HistoryStorage:
         revid6 = self._dostore(oid, revid=revid5, data=MinPO(16),
                                version=version)
         # Now commit the version
-        self._storage.tpc_begin(self._transaction)
-        oids = self._storage.abortVersion(version, self._transaction)
-        self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_finish(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        oids = self._storage.abortVersion(version, t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
         # After consultation with Jim, we agreed that the semantics of
         # revision id's after a version commit is that the committed object
         # gets a new serial number (a.k.a. revision id).  Note that

src/ZODB/tests/StorageTestBase.py

@@ -147,14 +147,15 @@ class StorageTestBase(unittest.TestCase):
         if version is None:
             version = ''
         # Begin the transaction
-        self._transaction = Transaction()
-        self._storage.tpc_begin(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        self._transaction = t
         # Store an object
-        r1 = self._storage.store(oid, revid, data, version,
-                                       self._transaction)
+        r1 = self._storage.store(oid, revid, data, version, t)
         # Finish the transaction
-        r2 = self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_finish(self._transaction)
+        r2 = self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
+        self._transaction = None
         return handle_serials(oid, r1, r2)
         
     def _dostoreNP(self, oid=None, revid=None, data=None, version=None):

src/ZODB/tests/Synchronization.py

@@ -67,13 +67,13 @@ class SynchronizedStorage:
         apply(self.assertRaises, args)
 
     def verifyWrongTrans(self, callable, *args):
-        self._storage.tpc_begin(self._transaction)
+        self._storage.tpc_begin(Transaction())
         args = (StorageTransactionError, callable) + args
         apply(self.assertRaises, args)
 
     def checkAbortVersionNotCommitting(self):
         self.verifyNotCommitting(self._storage.abortVersion,
-                                 VERSION, self._transaction)
+                                 VERSION, Transaction())
 
     def checkAbortVersionWrongTrans(self):
         self.verifyWrongTrans(self._storage.abortVersion,
@@ -81,7 +81,7 @@ class SynchronizedStorage:
 
     def checkCommitVersionNotCommitting(self):
         self.verifyNotCommitting(self._storage.commitVersion,
-                                 VERSION, "", self._transaction)
+                                 VERSION, "", Transaction())
 
     def checkCommitVersionWrongTrans(self):
         self.verifyWrongTrans(self._storage.commitVersion,
@@ -90,7 +90,7 @@ class SynchronizedStorage:
 
     def checkStoreNotCommitting(self):
         self.verifyNotCommitting(self._storage.store,
-                                 OID, SERIALNO, "", "", self._transaction)
+                                 OID, SERIALNO, "", "", Transaction())
     
     def checkStoreWrongTrans(self):
         self.verifyWrongTrans(self._storage.store,
@@ -107,18 +107,19 @@ class SynchronizedStorage:
         self._storage.tpc_abort(Transaction())
 
     def checkAbortWrongTrans(self):
-        self._storage.tpc_begin(self._transaction)
+        self._storage.tpc_begin(Transaction())
         self._storage.tpc_abort(Transaction())
 
     def checkFinishNotCommitting(self):
         self._storage.tpc_finish(Transaction())
 
     def checkFinishWrongTrans(self):
-        self._storage.tpc_begin(self._transaction)
+        self._storage.tpc_begin(Transaction())
         self._storage.tpc_finish(Transaction())
     
     def checkBeginCommitting(self):
-        self._storage.tpc_begin(self._transaction)
-        self._storage.tpc_begin(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        self._storage.tpc_begin(t)
 
     # XXX how to check undo?

src/ZODB/tests/TransactionalUndoVersionStorage.py

@@ -2,6 +2,7 @@
 # that supports both transactionalUndo() and versions must pass these tests.
 
 from ZODB import POSException
+from ZODB.Transaction import Transaction
 from ZODB.tests.MinPO import MinPO
 from ZODB.tests.StorageTestBase import zodb_unpickle
 
@@ -11,14 +12,17 @@ class TransactionalUndoVersionStorage:
         oid = self._storage.new_oid()
         version = 'one'
         revid_a = self._dostore(oid, data=MinPO(91))
-        revid_b = self._dostore(oid, revid=revid_a, data=MinPO(92), version=version)
-        revid_c = self._dostore(oid, revid=revid_b, data=MinPO(93), version=version)
+        revid_b = self._dostore(oid, revid=revid_a, data=MinPO(92),
+                                version=version)
+        revid_c = self._dostore(oid, revid=revid_b, data=MinPO(93),
+                                version=version)
         info=self._storage.undoInfo()
         tid=info[0]['id']
-        self._storage.tpc_begin(self._transaction)
-        oids = self._storage.transactionalUndo(tid, self._transaction)
-        self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_finish(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        oids = self._storage.transactionalUndo(tid, t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
         assert len(oids) == 1
         assert oids[0] == oid
         data, revid = self._storage.load(oid, '')
@@ -28,10 +32,11 @@ class TransactionalUndoVersionStorage:
         assert revid > revid_b and revid > revid_c
         assert zodb_unpickle(data) == MinPO(92)
         # Now commit the version...
-        self._storage.tpc_begin(self._transaction)
-        oids = self._storage.commitVersion(version, '', self._transaction)
-        self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_finish(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        oids = self._storage.commitVersion(version, '', t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
         assert len(oids) == 1
         assert oids[0] == oid
 
@@ -46,10 +51,11 @@ class TransactionalUndoVersionStorage:
         # ...and undo the commit
         info=self._storage.undoInfo()
         tid=info[0]['id']
-        self._storage.tpc_begin(self._transaction)
-        oids = self._storage.transactionalUndo(tid, self._transaction)
-        self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_finish(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        oids = self._storage.transactionalUndo(tid, t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
         assert len(oids) == 1
         assert oids[0] == oid
         data, revid = self._storage.load(oid, version)
@@ -57,10 +63,11 @@ class TransactionalUndoVersionStorage:
         data, revid = self._storage.load(oid, '')
         assert zodb_unpickle(data) == MinPO(91)
         # Now abort the version
-        self._storage.tpc_begin(self._transaction)
-        oids = self._storage.abortVersion(version, self._transaction)
-        self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_finish(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        oids = self._storage.abortVersion(version, t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
         assert len(oids) == 1
         assert oids[0] == oid
         # The object should not exist in the version now, but it should exist
@@ -76,10 +83,11 @@ class TransactionalUndoVersionStorage:
         # Now undo the abort
         info=self._storage.undoInfo()
         tid=info[0]['id']
-        self._storage.tpc_begin(self._transaction)
-        oids = self._storage.transactionalUndo(tid, self._transaction)
-        self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_finish(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        oids = self._storage.transactionalUndo(tid, t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
         assert len(oids) == 1
         assert oids[0] == oid
         # And the object should be back in versions 'one' and ''

src/ZODB/tests/VersionStorage.py

@@ -8,6 +8,7 @@ Any storage that supports versions should be able to pass all these tests.
 # code.  Barry and Jeremy didn't understand versions then.
 
 from ZODB import POSException
+from ZODB.Transaction import Transaction
 from ZODB.tests.MinPO import MinPO
 from ZODB.tests.StorageTestBase import zodb_unpickle
 
@@ -144,10 +145,11 @@ class VersionStorage:
         
 ##        s1 = self._storage.getSerial(oid)
         # Now abort the version -- must be done in a transaction
-        self._storage.tpc_begin(self._transaction)
-        oids = self._storage.abortVersion(version, self._transaction)
-        self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_finish(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        oids = self._storage.abortVersion(version, t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
 ##        s2 = self._storage.getSerial(oid)
 ##        eq(s1, s2) # or self.assert(s2 > s1) ?
         eq(len(oids), 1)
@@ -159,14 +161,15 @@ class VersionStorage:
         eq = self.assertEqual
         oid, version = self._setup_version()
         # Now abort a bogus version
-        self._storage.tpc_begin(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
 
         #JF# The spec is silent on what happens if you abort or commit
         #JF# a non-existent version. FileStorage consideres this a noop.
         #JF# We can change the spec, but until we do ....
         #JF# self.assertRaises(POSException.VersionError,
         #JF#                   self._storage.abortVersion,
-        #JF#                   'bogus', self._transaction)
+        #JF#                   'bogus', t)
 
         # And try to abort the empty version
         if (hasattr(self._storage, 'supportsTransactionalUndo')
@@ -174,12 +177,12 @@ class VersionStorage:
             # XXX FileStorage used to be broken on this one
             self.assertRaises(POSException.VersionError,
                               self._storage.abortVersion,
-                              '', self._transaction)
+                              '', t)
         
         # But now we really try to abort the version
-        oids = self._storage.abortVersion(version, self._transaction)
-        self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_finish(self._transaction)
+        oids = self._storage.abortVersion(version, t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
         eq(len(oids), 1)
         eq(oids[0], oid)
         data, revid = self._storage.load(oid, '')
@@ -194,19 +197,21 @@ class VersionStorage:
         oid1, version1 = self._setup_version('one')
         data, revid1 = self._storage.load(oid1, version1)
         eq(zodb_unpickle(data), MinPO(54))
-        self._storage.tpc_begin(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
         self.assertRaises(POSException.VersionCommitError,
                           self._storage.commitVersion,
-                          'one', 'one', self._transaction)
+                          'one', 'one', t)
 
     def checkModifyAfterAbortVersion(self):
         eq = self.assertEqual
         oid, version = self._setup_version()
         # Now abort the version
-        self._storage.tpc_begin(self._transaction)
-        oids = self._storage.abortVersion(version, self._transaction)
-        self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_finish(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        oids = self._storage.abortVersion(version, t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
         # Load the object's current state (which gets us the revid)
         data, revid = self._storage.load(oid, '')
         # And modify it a few times
@@ -225,10 +230,11 @@ class VersionStorage:
         data, revid = self._storage.load(oid, '')
         eq(zodb_unpickle(data), MinPO(51))
         # Try committing this version to the empty version
-        self._storage.tpc_begin(self._transaction)
-        oids = self._storage.commitVersion(version, '', self._transaction)
-        self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_finish(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        oids = self._storage.commitVersion(version, '', t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
         data, revid = self._storage.load(oid, '')
         eq(zodb_unpickle(data), MinPO(54))
 
@@ -251,11 +257,12 @@ class VersionStorage:
         eq(zodb_unpickle(data), MinPO(51))
         
         # Okay, now let's commit object1 to version2
-        self._storage.tpc_begin(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
         oids = self._storage.commitVersion(version1, version2,
-                                           self._transaction)
-        self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_finish(self._transaction)
+                                           t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
         eq(len(oids), 1)
         eq(oids[0], oid1)
         data, revid = self._storage.load(oid1, version2)
@@ -286,10 +293,11 @@ class VersionStorage:
         eq(zodb_unpickle(data), MinPO(51))
 
         # First, let's abort version1
-        self._storage.tpc_begin(self._transaction)
-        oids = self._storage.abortVersion(version1, self._transaction)
-        self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_finish(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        oids = self._storage.abortVersion(version1, t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
         eq(len(oids), 1)
         eq(oids[0], oid1)
         data, revid = self._storage.load(oid1, '')
@@ -310,10 +318,11 @@ class VersionStorage:
         data, revid = self._storage.load(oid2, version2)
         eq(zodb_unpickle(data), MinPO(54))
         # Okay, now let's commit version2 back to the trunk
-        self._storage.tpc_begin(self._transaction)
-        oids = self._storage.commitVersion(version2, '', self._transaction)
-        self._storage.tpc_vote(self._transaction)
-        self._storage.tpc_finish(self._transaction)
+        t = Transaction()
+        self._storage.tpc_begin(t)
+        oids = self._storage.commitVersion(version2, '', t)
+        self._storage.tpc_vote(t)
+        self._storage.tpc_finish(t)
         eq(len(oids), 1)
         eq(oids[0], oid2)
         data, revid = self._storage.load(oid1, '')