[Zope-Checkins] CVS: ZODB/src/ZODB - Connection.py:1.145 DB.py:1.73

[Tim Peters]

Update of /cvs-repository/ZODB/src/ZODB
In directory cvs.zope.org:/tmp/cvs-serv14718/src/ZODB

Modified Files:
	Connection.py DB.py 
Log Message:
Close a thread race in Connection.close(), diagnosed by Marius Gedminas.
DB._closeConnection() sets the connection's _db member to None now, under
protection of the lock it holds anyway.  At a deeper level, it's unclear
why _db keeps getting set and unset to begin with, but no time for that
now (and there are already XXX comments about it in the code).

Alas, I wasn't able to write a test that provoked the original bug in
finite time (it's a tiny timing hole), except via calling sleep() between
two lines that don't exist anymore.  Good enough.


=== ZODB/src/ZODB/Connection.py 1.144 => 1.145 ===
--- ZODB/src/ZODB/Connection.py:1.144	Tue Apr  6 17:47:12 2004
+++ ZODB/src/ZODB/Connection.py	Thu Apr  8 14:12:25 2004
@@ -466,7 +466,9 @@
         # Return the connection to the pool.
         if self._db is not None:
             self._db._closeConnection(self)
-            self._db = None
+            # _closeConnection() set self._db to None.  However, we can't
+            # assert that here, because self may have been reused (by
+            # another thread) by the time we get back here.
 
     def commit(self, obj, transaction):
         if obj is self:


=== ZODB/src/ZODB/DB.py 1.72 => 1.73 ===
--- ZODB/src/ZODB/DB.py:1.72	Wed Mar 31 22:56:58 2004
+++ ZODB/src/ZODB/DB.py	Thu Apr  8 14:12:25 2004
@@ -72,7 +72,7 @@
         setCacheDeactivateAfter,
         getVersionCacheDeactivateAfter, setVersionCacheDeactivateAfter
     """
-    
+
     klass = Connection  # Class to use for connections
     _activity_monitor = None
 
@@ -163,14 +163,21 @@
         return m
 
     def _closeConnection(self, connection):
-        """Return a connection to the pool"""
+        """Return a connection to the pool.
+
+        connection._db must be self on entry.
+        """
+
         self._a()
         try:
+            assert connection._db is self
+            connection._db = None
+
             am = self._activity_monitor
             if am is not None:
                 am.closedConnection(connection)
-            version=connection._version
-            pools,pooll=self._pools
+            version = connection._version
+            pools, pooll = self._pools
             try:
                 pool, allocated, pool_lock = pools[version]
             except KeyError:
@@ -184,7 +191,7 @@
                 return
 
             pool.append(connection)
-            if len(pool)==1:
+            if len(pool) == 1:
                 # Pool now usable again, unlock it.
                 pool_lock.release()
         finally:
@@ -708,7 +715,7 @@
     def __init__(self, db, version):
         super(AbortVersion, self).__init__(db)
         self._version = version
-        
+
     def commit(self, ob, t):
         tid, oids = self._db._storage.abortVersion(self._version, t)
         self._db.invalidate(tid, dict.fromkeys(oids, 1), version=self._version)