From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Thu, 4 Jul 2013 17:18:00 +0000 (-0400)
Subject: Fixed bug where :class:`.QueuePool` would lose the correct
X-Git-Tag: rel_0_9_0b1~196
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=f390639bf1a7a5a2a47bcd6df7106cf5855a44c1;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Fixed bug where :class:`.QueuePool` would lose the correct
checked out count if an existing pooled connection failed to reconnect
after an invalidate or recycle event.  Also in 0.8.3.
[ticket:2772]
---

diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst
index 1d52a994d3..04c7f1f31a 100644
--- a/doc/build/changelog/changelog_08.rst
+++ b/doc/build/changelog/changelog_08.rst
@@ -3,6 +3,17 @@
 0.8 Changelog
 ==============
 
+.. changelog::
+    :version: 0.8.3
+
+    .. change::
+        :tags: bug, engine, pool
+        :tickets: 2772
+
+        Fixed bug where :class:`.QueuePool` would lose the correct
+        checked out count if an existing pooled connection failed to reconnect
+        after an invalidate or recycle event.
+
 .. changelog::
     :version: 0.8.2
     :released: July 3, 2013
diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index 2b33a5cb10..c2abc2504d 100644
--- a/doc/build/changelog/changelog_09.rst
+++ b/doc/build/changelog/changelog_09.rst
@@ -6,6 +6,14 @@
 .. changelog::
     :version: 0.9.0
 
+    .. change::
+        :tags: bug, engine, pool
+        :tickets: 2772
+
+        Fixed bug where :class:`.QueuePool` would lose the correct
+        checked out count if an existing pooled connection failed to reconnect
+        after an invalidate or recycle event.  Also in 0.8.3.
+
     .. change::
         :tags: bug, mysql
         :tickets: 2768
diff --git a/lib/sqlalchemy/pool.py b/lib/sqlalchemy/pool.py
index 97411dd3a3..498b001c15 100644
--- a/lib/sqlalchemy/pool.py
+++ b/lib/sqlalchemy/pool.py
@@ -328,7 +328,11 @@ class _ConnectionRecord(object):
     @classmethod
     def checkout(cls, pool):
         rec = pool._do_get()
-        dbapi_connection = rec.get_connection()
+        try:
+            dbapi_connection = rec.get_connection()
+        except:
+            rec.checkin()
+            raise
         fairy = _ConnectionFairy(dbapi_connection, rec)
         rec.fairy_ref = weakref.ref(
                         fairy,
@@ -565,6 +569,7 @@ class _ConnectionFairy(object):
             _refs.remove(self._connection_record)
             self._connection_record.fairy_ref = None
             self._connection_record.connection = None
+            # TODO: should this be _return_conn?
             self._pool._do_return_conn(self._connection_record)
             self.info = self.info.copy()
             self._connection_record = None
diff --git a/test/engine/test_pool.py b/test/engine/test_pool.py
index 8c4bcd8b5a..3e0188bd70 100644
--- a/test/engine/test_pool.py
+++ b/test/engine/test_pool.py
@@ -14,11 +14,20 @@ def MockDBAPI():
     def cursor():
         while True:
             yield Mock()
+
     def connect():
         while True:
             yield Mock(cursor=Mock(side_effect=cursor()))
 
-    return Mock(connect=Mock(side_effect=connect()))
+    def shutdown(value):
+        if value:
+            db.connect = Mock(side_effect=Exception("connect failed"))
+        else:
+            db.connect = Mock(side_effect=connect())
+
+    db = Mock(connect=Mock(side_effect=connect()),
+                    shutdown=shutdown, _shutdown=False)
+    return db
 
 class PoolTestBase(fixtures.TestBase):
     def setup(self):
@@ -1073,6 +1082,46 @@ class QueuePoolTest(PoolTestBase):
         c3 = p.connect()
         assert id(c3.connection) != c_id
 
+    def _assert_cleanup_on_pooled_reconnect(self, dbapi, p):
+        # p is QueuePool with size=1, max_overflow=2,
+        # and one connection in the pool that will need to
+        # reconnect when next used (either due to recycle or invalidate)
+        eq_(p.checkedout(), 0)
+        eq_(p._overflow, 0)
+        dbapi.shutdown(True)
+        assert_raises(
+            Exception,
+            p.connect
+        )
+        eq_(p._overflow, 0)
+        eq_(p.checkedout(), 0)  # and not 1
+
+        dbapi.shutdown(False)
+
+        c1 = p.connect()
+        assert p._pool.empty()  # poolsize is one, so we're empty OK
+        c2 = p.connect()
+        eq_(p._overflow, 1)  # and not 2
+
+        # this hangs if p._overflow is 2
+        c3 = p.connect()
+
+    def test_error_on_pooled_reconnect_cleanup_invalidate(self):
+        dbapi, p = self._queuepool_dbapi_fixture(pool_size=1, max_overflow=2)
+        c1 = p.connect()
+        c1.invalidate()
+        c1.close()
+        self._assert_cleanup_on_pooled_reconnect(dbapi, p)
+
+    def test_error_on_pooled_reconnect_cleanup_recycle(self):
+        dbapi, p = self._queuepool_dbapi_fixture(pool_size=1,
+                                        max_overflow=2, recycle=1)
+        c1 = p.connect()
+        c1.close()
+        time.sleep(1)
+        self._assert_cleanup_on_pooled_reconnect(dbapi, p)
+
+
     def test_invalidate(self):
         p = self._queuepool_fixture(pool_size=1, max_overflow=0)
         c1 = p.connect()