- 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]

3 files changed, 59 insertions, 2 deletions

diff --git a/CHANGES b/CHANGES
index 91da615d0..60a09b2ea 100644
--- 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 fa9582010..a5b06f63a 100644
--- 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 2df8b9ca2..d0c7ca7b9 100644
--- 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()