From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Wed, 23 Mar 2011 03:44:38 +0000 (-0400)
Subject: - Fixed bug in QueuePool, SingletonThreadPool whereby
X-Git-Tag: rel_0_6_7~15
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b81838a46d2016f3806bba1d3c0dea49ce2213c5;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

- Fixed bug in QueuePool, SingletonThreadPool whereby
connections that were discarded via overflow or periodic
cleanup() were not explicitly closed, leaving garbage
collection to the task instead.   This generally only
affects non-reference-counting backends like Jython
and Pypy.  Thanks to Jaimy Azle for spotting
this.  [ticket:2102]
---

diff --git a/CHANGES b/CHANGES
index 397845ab86..d7822261a3 100644
--- a/CHANGES
+++ b/CHANGES
@@ -62,6 +62,15 @@ CHANGES
   - Added accessors to ResultProxy "returns_rows", "is_insert"
     [ticket:2089]
 
+- engine
+  - Fixed bug in QueuePool, SingletonThreadPool whereby
+    connections that were discarded via overflow or periodic
+    cleanup() were not explicitly closed, leaving garbage 
+    collection to the task instead.   This generally only
+    affects non-reference-counting backends like Jython
+    and Pypy.  Thanks to Jaimy Azle for spotting
+    this.  [ticket:2102]
+
 - postgresql
   - When explicit sequence execution derives the name 
     of the auto-generated sequence of a SERIAL column, 
diff --git a/lib/sqlalchemy/pool.py b/lib/sqlalchemy/pool.py
index 13b1092d16..31ed6b5126 100644
--- a/lib/sqlalchemy/pool.py
+++ b/lib/sqlalchemy/pool.py
@@ -580,7 +580,8 @@ class SingletonThreadPool(Pool):
 
     def cleanup(self):
         while len(self._all_conns) > self.size:
-            self._all_conns.pop()
+            c = self._all_conns.pop()
+            c.close()
 
     def status(self):
         return "SingletonThreadPool id:%d size: %d" % \
@@ -694,6 +695,7 @@ class QueuePool(Pool):
         try:
             self._pool.put(conn, False)
         except sqla_queue.Full:
+            conn.close()
             if self._overflow_lock is None:
                 self._overflow -= 1
             else:
diff --git a/test/engine/test_pool.py b/test/engine/test_pool.py
index df9aa7703b..7e30a6c6e8 100644
--- a/test/engine/test_pool.py
+++ b/test/engine/test_pool.py
@@ -606,6 +606,32 @@ class QueuePoolTest(PoolTestBase):
         lazy_gc()
         assert not pool._refs
 
+    def test_overflow_no_gc_tlocal(self):
+        self._test_overflow_no_gc(True)
+
+    def test_overflow_no_gc(self):
+        self._test_overflow_no_gc(False)
+
+    def _test_overflow_no_gc(self, threadlocal):
+        p = pool.QueuePool(creator=mock_dbapi.connect, pool_size=2,
+                           max_overflow=2, use_threadlocal=False)
+
+        # disable weakref collection of the 
+        # underlying connections
+        strong_refs = set()
+        def _conn():
+            c = p.connect()
+            strong_refs.add(c.connection)
+            return c
+
+        for j in xrange(5):
+            conns = [_conn() for i in xrange(4)]
+            for c in conns:
+                c.close()
+
+        still_opened = len([c for c in strong_refs if not c.closed])
+        eq_(still_opened, 2)
+
     def test_weakref_kaboom(self):
         p = pool.QueuePool(creator=mock_dbapi.connect, pool_size=3,
                            max_overflow=-1, use_threadlocal=True)
@@ -731,15 +757,31 @@ class QueuePoolTest(PoolTestBase):
 class SingletonThreadPoolTest(PoolTestBase):
 
     def test_cleanup(self):
+        self._test_cleanup(False)
+
+    def test_cleanup_no_gc(self):
+        self._test_cleanup(True)
+
+    def _test_cleanup(self, strong_refs):
         """test that the pool's connections are OK after cleanup() has
         been called."""
 
         p = pool.SingletonThreadPool(creator=mock_dbapi.connect,
                 pool_size=3)
 
+        if strong_refs:
+            sr = set()
+            def _conn():
+                c = p.connect()
+                sr.add(c.connection)
+                return c
+        else:
+            def _conn():
+                return p.connect()
+
         def checkout():
             for x in xrange(10):
-                c = p.connect()
+                c = _conn()
                 assert c
                 c.cursor()
                 c.close()
@@ -754,6 +796,10 @@ class SingletonThreadPoolTest(PoolTestBase):
             th.join()
         assert len(p._all_conns) == 3
 
+        if strong_refs:
+            still_opened = len([c for c in sr if not c.closed])
+            eq_(still_opened, 3)
+
 class NullPoolTest(PoolTestBase):
     def test_reconnect(self):
         dbapi = MockDBAPI()