From a294df9748f1314a2db7de5bc39d091854d0fec3 Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Tue, 22 Mar 2011 23:39:05 -0400
Subject: [PATCH] - 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]

---
 CHANGES                  |  9 ++++++++
 lib/sqlalchemy/pool.py   |  4 +++-
 test/engine/test_pool.py | 48 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/CHANGES b/CHANGES
index 91da615d00..60a09b2ea1 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 fa95820108..a5b06f63ae 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 2df8b9ca29..d0c7ca7b94 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()
-- 
2.47.2