Fix obscure bug.

If a threaded transaction manager ever passed None to
the Transaction constructor's `synchronizers` argument, then
synchronizers registered later in the same transaction
were invisible to the transaction, and so their afterCompletion()
methods wouldn't get called when the transaction ended.

NEWS.txt

+What's new in ZODB3 3.4a7?
+==========================
+Release date: 06-May-2005
+
+This was an internal release, to fix an obscure older bug discovered while
+testing the ``ISynchronizer`` enhancements.
+
+transaction
+-----------
+
+- If the first activity seen by a new ``ThreadTransactionManager`` was
+  an explicit ``begin()`` call, then synchronizers registered after that
+  (but still during the first transaction) were not communicated to the
+  transaction object.  As a result, the ``afterCompletion()`` methods of
+  registered synchronizers weren't called when the first transaction ended.
+
+
 What's new in ZODB3 3.4a6?
 ==========================
 Release date: 05-May-2005

src/ZODB/tests/synchronizers.txt

@@ -50,7 +50,6 @@ newTransaction() hook.
     >>> st.sync_called  # False before 3.4
     True
 
-
 Clean up.  Closing db isn't enough -- closing a DB doesn't close its
 Connections.  Leaving our Connection open here can cause the
 SimpleStorage.sync() method to get called later, during another test, and
@@ -58,4 +57,46 @@ our doctest-synthesized module globals no longer exist then.  You get
 a weird traceback then ;-)
 
     >>> cn.close()
+
+One more, very obscure.  It was the case that if the first action a new
+threaded transaction manager saw was a begin() call, then synchronizers
+registered after that in the same transaction weren't communicated to
+the Transaction object, and so the storage's afterCompletion() hook wasn't
+called when the transaction commited.  None of the test suites (ZODB's,
+Zope 2.8's, or Zope3's) caught that, but apparently Zope3 takes this path
+at some point when serving pages.
+
+    >>> tm = transaction.ThreadTransactionManager()
+    >>> st.sync_called = False
+    >>> dummy = tm.begin()  # we're doing this _before_ opening a connection
+    >>> cn = db.open(txn_mgr=tm)
+    >>> rt = cn.root()      # make a change
+    >>> rt['c'] = 3
+    >>> st.sync_called
+    False
+
+Now ensure that st.afterCompletion() gets called by commit despite that the
+Connection registered after the transaction began:
+
+    >>> tm.commit()
+    >>> st.sync_called
+    True
+
+And try the same thing with a non-threaded TM:
+
+    >>> cn.close()
+    >>> tm = transaction.TransactionManager()
+    >>> st.sync_called = False
+    >>> dummy = tm.begin()  # we're doing this _before_ opening a connection
+    >>> cn = db.open(txn_mgr=tm)
+    >>> rt = cn.root()      # make a change
+    >>> rt['d'] = 4
+    >>> st.sync_called
+    False
+    >>> tm.commit()
+    >>> st.sync_called
+    True
+
+    >>> cn.close()
     >>> db.close()
+

src/transaction/_manager.py

@@ -33,7 +33,7 @@ from transaction._transaction import Transaction
 # at top level here.
 
 # Call the ISynchronizer newTransaction() method on every element of
-# WeakSet synchs (or skip it if synchs is None).
+# WeakSet synchs.
 # A transaction manager needs to do this whenever begin() is called.
 # Since it would be good if tm.get() returned the new transaction while
 # newTransaction() is running, calling this has to be delayed until after
@@ -43,6 +43,13 @@ def _new_transaction(txn, synchs):
     if synchs:
         synchs.map(lambda s: s.newTransaction(txn))
 
+# Important:  we must always pass a WeakSet (even if empty) to the Transaction
+# constructor:  synchronizers are registered with the TM, but the
+# ISynchronizer xyzCompletion() methods are called by Transactions without
+# consulting the TM, so we need to pass a mutable collection of synchronizers
+# so that Transactions "see" synchronizers that get registered after the
+# Transaction object is constructed.
+
 class TransactionManager(object):
 
     def __init__(self):
@@ -91,6 +98,7 @@ class ThreadTransactionManager(TransactionManager):
     def __init__(self):
         # _threads maps thread ids to transactions
         self._txns = {}
+
         # _synchs maps a thread id to a WeakSet of registered synchronizers.
         # The WeakSet is passed to the Transaction constructor, because the
         # latter needs to call the synchronizers when it commits.
@@ -101,7 +109,12 @@ class ThreadTransactionManager(TransactionManager):
         txn = self._txns.get(tid)
         if txn is not None:
             txn.abort()
+
         synchs = self._synchs.get(tid)
+        if synchs is None:
+            from ZODB.utils import WeakSet
+            synchs = self._synchs[tid] = WeakSet()
+
         txn = self._txns[tid] = Transaction(synchs, self)
         _new_transaction(txn, synchs)
         return txn
@@ -111,6 +124,9 @@ class ThreadTransactionManager(TransactionManager):
         txn = self._txns.get(tid)
         if txn is None:
             synchs = self._synchs.get(tid)
+            if synchs is None:
+                from ZODB.utils import WeakSet
+                synchs = self._synchs[tid] = WeakSet()
             txn = self._txns[tid] = Transaction(synchs, self)
         return txn
 
@@ -120,11 +136,10 @@ class ThreadTransactionManager(TransactionManager):
         del self._txns[tid]
 
     def registerSynch(self, synch):
-        from ZODB.utils import WeakSet
-
         tid = thread.get_ident()
         ws = self._synchs.get(tid)
         if ws is None:
+            from ZODB.utils import WeakSet
             ws = self._synchs[tid] = WeakSet()
         ws.add(synch)