[Zodb-checkins] CVS: StandaloneZODB/ZEO - ClientStorage.py:1.47

[Tim Peters]

Update of /cvs-repository/StandaloneZODB/ZEO
In directory cvs.zope.org:/tmp/cvs-serv29742/ZEO

Modified Files:
	ClientStorage.py 
Log Message:
tpc_begin():  self._transaction wasn't protected by the condvar at all
anymore.  Tried to fix that.  Jeremy, please review the new XXX comments.


=== StandaloneZODB/ZEO/ClientStorage.py 1.46 => 1.47 ===
--- StandaloneZODB/ZEO/ClientStorage.py:1.46	Fri Aug 16 14:15:04 2002
+++ StandaloneZODB/ZEO/ClientStorage.py	Fri Aug 16 17:42:49 2002
@@ -2,14 +2,14 @@
 #
 # Copyright (c) 2001, 2002 Zope Corporation and Contributors.
 # All Rights Reserved.
-# 
+#
 # This software is subject to the provisions of the Zope Public License,
 # Version 2.0 (ZPL).  A copy of the ZPL should accompany this distribution.
 # THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
 # WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
 # WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
 # FOR A PARTICULAR PURPOSE
-# 
+#
 ##############################################################################
 """Network ZODB storage client
 
@@ -117,7 +117,7 @@
         # Mutual exclusion is achieved using tpc_cond(), which
         # protects _transaction.  A thread that wants to assign to
         # self._transaction must acquire tpc_cond() first.
-        
+
         # Invariant: If self._transaction is not None, then tpc_cond()
         # must be acquired.
         self.tpc_cond = threading.Condition()
@@ -339,22 +339,20 @@
     def tpc_begin(self, transaction, tid=None, status=' '):
         self.tpc_cond.acquire()
         while self._transaction is not None:
+            # It is allowable for a client to call two tpc_begins in a
+            # row with the same transaction, and the second of these
+            # must be ignored.
             if self._transaction == transaction:
-                # Our tpc_cond lock is re-entrant.  It is allowable for a
-                # client to call two tpc_begins in a row with the same
-                # transaction, and the second of these must be ignored.  Our
-                # locking is safe because the acquire() above gives us a
-                # second lock on tpc_cond, and the following release() brings
-                # us back to owning just the one tpc_cond lock (acquired
-                # during the first of two consecutive tpc_begins).
                 self.tpc_cond.release()
                 return
             self.tpc_cond.wait()
-        self.tpc_cond.release()
 
         if self._server is None:
-            self.tpc_cond.release()
+            # XXX Why set _transaction to None?  It must be None now, else
+            # XXX we would have stayed in the while loop.
+            assert self._transaction is None
             self._transaction = None
+            self.tpc_cond.release()
             raise ClientDisconnected()
 
         if tid is None:
@@ -363,7 +361,13 @@
         else:
             self._ts = TimeStamp(tid)
             id = tid
+        # XXX Can setting _transaction be moved above the "id=" business?
+        # XXX We want to hold the condvar across as little code as possible,
+        # XXX to slash the chances for deadlock (among other things); e.g.,
+        # XXX if one of those timestamp routines raised an exception, we'd
+        # XXX hold the condvar forever.
         self._transaction = transaction
+        self.tpc_cond.release()
 
         try:
             r = self._server.tpc_begin(id,
@@ -373,10 +377,10 @@
                                        tid, status)
         except:
             # Client may have disconnected during the tpc_begin().
-            # Then notifyDisconnected() will have released the lock.
             if self._server is not disconnected_stub:
                 self.tpc_cond.acquire()
                 self._transaction = None
+                self.tpc_cond.notify()
                 self.tpc_cond.release()
             raise