- 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 91da615d00109a20f73f078e5b80f87bd5520fd4..60a09b2ea1a2e53d107c0bb94663bc9844d4ed3b 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -514,6 +514,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 fa95820108e4eae012b18ffc6c046396750910da..a5b06f63aeacdcad86281cba0243036c9f48c858 100644 (file)
--- a/lib/sqlalchemy/pool.py
+++ b/lib/sqlalchemy/pool.py
@@ -536,7 +536,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" % \
@@ -655,6 +656,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 2df8b9ca292667c319d0a196bb6d8a988c0810d3..d0c7ca7b943bf31af212b3bd169d50e1d8952e39 100644 (file)
--- a/test/engine/test_pool.py
+++ b/test/engine/test_pool.py
@@ -804,6 +804,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 = self._queuepool_fixture(pool_size=2,
+                           max_overflow=2)
+
+        # 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 = self._queuepool_fixture(pool_size=3,
                            max_overflow=-1, use_threadlocal=True)
@@ -916,6 +942,12 @@ 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."""
 
@@ -923,9 +955,19 @@ class SingletonThreadPoolTest(PoolTestBase):
         p = pool.SingletonThreadPool(creator=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()
@@ -940,6 +982,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 AssertionPoolTest(PoolTestBase):
     def test_connect_error(self):
         dbapi = MockDBAPI()