[Zope-Checkins] CVS: ZODB3/ZEO - ClientStorage.py:1.73.2.17

[Tim Peters]

Update of /cvs-repository/ZODB3/ZEO
In directory cvs.zope.org:/tmp/cvs-serv27626/ZEO

Modified Files:
      Tag: ZODB3-3_1-branch
	ClientStorage.py 
Log Message:
load() and tpc_finish():  The new locking strategy was vulnerable to
deadlock, when e.g. an invalidation came in *during* a zeoLoad call.
The locks now only seek to make [updating the cache] mutually exclusive
with [receiving invalidations].


=== ZODB3/ZEO/ClientStorage.py 1.73.2.16 => 1.73.2.17 ===
--- ZODB3/ZEO/ClientStorage.py:1.73.2.16	Thu Jun  5 15:45:07 2003
+++ ZODB3/ZEO/ClientStorage.py	Thu Jun  5 16:33:37 2003
@@ -219,7 +219,7 @@
         # but _server is set only after cache verification has finished
         # and clients can safely use the server.  _pending_server holds
         # a server stub while it is being verified.
-        
+
         self._server = disconnected_stub
         self._connection = None
         self._pending_server = None
@@ -363,7 +363,7 @@
         # If there is no connection, return immediately.  Technically,
         # there are no pending invalidations so they are all handled.
         # There doesn't seem to be much benefit to raising an exception.
-        
+
         cn = self._connection
         if cn is not None:
             cn.pending()
@@ -608,25 +608,29 @@
         specified by the given object id and version, if they exist;
         otherwise a KeyError is raised.
         """
-        self._lock.acquire()
+        self._lock.acquire()    # for atomic processing of invalidations
         try:
             p = self._cache.load(oid, version)
             if p:
                 return p
-            if self._server is None:
-                raise ClientDisconnected()
-            p, s, v, pv, sv = self._server.zeoLoad(oid)
-            self._cache.checkSize(0)
-            self._cache.store(oid, p, s, v, pv, sv)
-            if v and version and v == version:
-                return pv, sv
-            else:
-                if s:
-                    return p, s
-                raise KeyError, oid # no non-version data for this
         finally:
             self._lock.release()
 
+        if self._server is None:
+            raise ClientDisconnected()
+
+        # If an invalidation for oid comes in during zeoLoad, that's OK
+        # because we'll get oid's new state.
+        p, s, v, pv, sv = self._server.zeoLoad(oid)
+        self._cache.checkSize(0)
+        self._cache.store(oid, p, s, v, pv, sv)
+        if v and version and v == version:
+            return pv, sv
+        else:
+            if s:
+                return p, s
+            raise KeyError, oid # no non-version data for this
+
     def modifiedInVersion(self, oid):
         """Storage API: return the version, if any, that modfied an object.
 
@@ -778,19 +782,20 @@
         if transaction is not self._transaction:
             return
         try:
-            self._lock.acquire()
+            self._lock.acquire()  # for atomic processing of invalidations
             try:
-                if f is not None:
-                    f()
-
-                self._server.tpc_finish(self._serial)
-
-                r = self._check_serials()
-                assert r is None or len(r) == 0, "unhandled serialnos: %s" % r
-
                 self._update_cache()
             finally:
                 self._lock.release()
+
+            if f is not None:
+                f()
+
+            self._server.tpc_finish(self._serial)
+
+            r = self._check_serials()
+            assert r is None or len(r) == 0, "unhandled serialnos: %s" % r
+
         finally:
             self.end_transaction()
 
@@ -801,7 +806,7 @@
         update or invalidate the cache.
         """
         # Must be called with _lock already acquired.
-        
+
         self._cache.checkSize(self._tbuf.get_size())
         try:
             self._tbuf.begin_iterate()