[Zodb-checkins] SVN: ZODB/trunk/src/ Bugs fixed

[Jim Fulton]

Log message for revision 117091:
  Bugs fixed
  ----------
  
  - When a pool timeout was specified for a database and old connections
    were removed due to timing out, an error occured due to a bug in the
    connection cleanup logic.
  
  - When mulri-database connections were no longer used and cleaned up,
    their subconnections weren't cleaned up properly.
  

Changed:
  U   ZODB/trunk/src/CHANGES.txt
  U   ZODB/trunk/src/ZODB/Connection.py
  U   ZODB/trunk/src/ZODB/DB.py
  U   ZODB/trunk/src/ZODB/tests/testDB.py

-=-
Modified: ZODB/trunk/src/CHANGES.txt
===================================================================
--- ZODB/trunk/src/CHANGES.txt	2010-09-30 14:26:02 UTC (rev 117090)
+++ ZODB/trunk/src/CHANGES.txt	2010-09-30 15:38:10 UTC (rev 117091)
@@ -12,7 +12,13 @@
   client led to a socket error. Now, ZEO clients treat '' as an alias
   for 'localhost'.
 
+- When a pool timeout was specified for a database and old connections
+  were removed due to timing out, an error occured due to a bug in the
+  connection cleanup logic.
 
+- When mulri-database connections were no longer used and cleaned up,
+  their subconnections weren't cleaned up properly.
+
 3.10.0b7 (2010-09-28)
 =====================
 

Modified: ZODB/trunk/src/ZODB/Connection.py
===================================================================
--- ZODB/trunk/src/ZODB/Connection.py	2010-09-30 14:26:02 UTC (rev 117090)
+++ ZODB/trunk/src/ZODB/Connection.py	2010-09-30 15:38:10 UTC (rev 117091)
@@ -1068,10 +1068,12 @@
         if getattr(self, '_reader', None) is not None:
             self._reader._cache = cache
 
-    def _releaseStorage(self):
-        """Tell the storage to release resources it's using"""
-        if self._mvcc_storage:
-            self._storage.release()
+    def _release_resources(self):
+        for c in self.connections.itervalues():
+            if c._mvcc_storage:
+                c._storage.release()
+            c._storage = c._normal_storage = None
+            c._cache = PickleCache(self, 0, 0)
 
     ##########################################################################
     # Python protocol

Modified: ZODB/trunk/src/ZODB/DB.py
===================================================================
--- ZODB/trunk/src/ZODB/DB.py	2010-09-30 14:26:02 UTC (rev 117090)
+++ ZODB/trunk/src/ZODB/DB.py	2010-09-30 15:38:10 UTC (rev 117091)
@@ -182,21 +182,7 @@
             ):
             t, c = available.pop(0)
             self.all.remove(c)
-            # While application code may still hold a reference to `c`,
-            # there's little useful that can be done with this Connection
-            # anymore. Its cache may be holding on to limited resources,
-            # and we replace the cache with an empty one now so that we
-            # don't have to wait for gc to reclaim it. Note that it's not
-            # possible for DB.open() to return `c` again: `c` can never be
-            # in an open state again.
-            # TODO: Perhaps it would be better to break the reference
-            # cycles between `c` and `c._cache`, so that refcounting
-            # reclaims both right now. But if user code _does_ have a
-            # strong reference to `c` now, breaking the cycle would not
-            # reclaim `c` now, and `c` would be left in a user-visible
-            # crazy state.
-            c._resetCache()
-            c._releaseStorage()
+            c._release_resources()
 
     def reduce_size(self):
         self._reduce_size()
@@ -226,14 +212,20 @@
         If a connection is no longer viable because it has timed out, it is
         garbage collected."""
         threshhold = time.time() - self.timeout
-        for t, c in list(self.available):
+
+        to_remove = ()
+        for (t, c) in self.available:
             if t < threshhold:
-                del self.available[t]
+                to_remove += (c,)
                 self.all.remove(c)
-                c._resetCache()
+                c._release_resources()
             else:
                 c.cacheGC()
 
+        if to_remove:
+            self.available[:] = [i for i in self.available
+                                 if i[1] not in to_remove]
+
 class KeyedConnectionPool(AbstractConnectionPool):
     # this pool keeps track of keyed connections all together.  It makes
     # it possible to make assertions about total numbers of keyed connections.
@@ -641,7 +633,7 @@
         def _(c):
             c.transaction_manager.abort()
             c.afterCompletion = c.newTransaction = c.close = noop
-            c._storage = c._normal_storage = None
+            c._release_resources()
 
         self.storage.close()
         del self.storage

Modified: ZODB/trunk/src/ZODB/tests/testDB.py
===================================================================
--- ZODB/trunk/src/ZODB/tests/testDB.py	2010-09-30 14:26:02 UTC (rev 117090)
+++ ZODB/trunk/src/ZODB/tests/testDB.py	2010-09-30 15:38:10 UTC (rev 117091)
@@ -324,8 +324,28 @@
     ...    "object you're saving is large.")
 
     >>> db.close()
+    """ # '
+
+def minimally_test_connection_timeout():
+    """There's a mechanism to discard old connections.
+
+    Make sure it doesn't error. :)
+
+    >>> db = ZODB.DB(None, pool_timeout=.01)
+    >>> c1 = db.open()
+    >>> c2 = db.open()
+    >>> c1.close()
+    >>> c2.close()
+    >>> time.sleep(.02)
+    >>> db.open() is c2
+    True
+
+    >>> db.pool.available
+    []
+
     """
 
+
 def test_suite():
     s = unittest.makeSuite(DBTests)
     s.addTest(doctest.DocTestSuite(