[Zodb-checkins] CVS: ZODB3/ZEO - ClientStorage.py:1.73.2.23.2.3

[Jeremy Hylton]

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

Modified Files:
      Tag: tim-loading_oids_status-branch
	ClientStorage.py 
Log Message:
Fix race condition in handling of invalidations.

Guarantee that an invalidation can't arrive between the status check
and the call _cache.store().

Refactor: move inc/dec status code into helper methods.


=== ZODB3/ZEO/ClientStorage.py 1.73.2.23.2.2 => 1.73.2.23.2.3 ===
--- ZODB3/ZEO/ClientStorage.py:1.73.2.23.2.2	Tue Jun 10 23:19:44 2003
+++ ZODB3/ZEO/ClientStorage.py	Wed Jun 11 12:03:30 2003
@@ -628,7 +628,44 @@
         if self._server is None:
             raise ClientDisconnected()
 
-        # XXX Race condition among load / invalid / store in cache
+        self._incLoadStatus(oid, version)
+
+        try:
+            p, s, v, pv, sv = self._server.zeoLoad(oid)
+        except:
+            self._lock.acquire()
+            try:
+                self._decLoadStatus(oid, version)
+            finally:
+                self._lock.release()
+            raise
+
+        self._lock.acquire()
+        try:
+            statv, statnv = self._decLoadStatus(oid, version)
+            if statv and statnv:
+                self._cache.checkSize(0)
+                self._cache.store(oid, p, s, v, pv, sv)
+            else:
+                if not statv:
+                    self._cache.invalidate(oid, version)
+                if not statnv:
+                    self._cache.invalidate(oid, '')
+        finally:
+            self._lock.release()
+                        
+        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 _incLoadStatus(self, oid, version):
+        """Increment the load count for oid, version pair.
+
+        Does its own locking.
+        """
         pairs = (oid, version), (oid, "")
         self._lock.acquire()
         try:
@@ -643,38 +680,26 @@
         finally:
             self._lock.release()
 
-        try:
-            p, s, v, pv, sv = self._server.zeoLoad(oid)
-        finally:
-            statii = [] # will be pair [version status, non-version status]
-            self._lock.acquire()
-            try:
-                for pair in pairs:
-                    count, status = self._loading_oids_status[pair]
-                    statii.append(status)
-                    count -= 1
-                    if count:
-                        self._loading_oids_status[pair] = count, status
-                    else:
-                        del self._loading_oids_status[pair]
-            finally:
-                self._lock.release()
-
-        if statii[0] and statii[1]: # both OK
-            self._cache.checkSize(0)
-            self._cache.store(oid, p, s, v, pv, sv)
-        else:
-            if not statii[0]:
-                self._cache.invalidate(oid, version)
-            if not statii[1]:
-                self._cache.invalidate(oid, '')
+    def _decLoadStatus(self, oid, version):
+        """Decrement load count and return statii.
 
-        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
+        Statii is a 2-element list of booleans indicating whether
+        the object, version was invalidated during the load.  The
+        first element is oid, version; the second element is oid, "".
+        
+        Caller must hold self._lock.
+        """
+        statii = []
+        pairs = (oid, version), (oid, "")
+        for pair in pairs:
+            count, status = self._loading_oids_status[pair]
+            statii.append(status)
+            count -= 1
+            if count:
+                self._loading_oids_status[pair] = count, status
+            else:
+                del self._loading_oids_status[pair]
+        return statii
 
     def modifiedInVersion(self, oid):
         """Storage API: return the version, if any, that modfied an object.