diff options
| -rw-r--r-- | doc/build/changelog/changelog_09.rst | 18 | ||||
| -rw-r--r-- | lib/sqlalchemy/pool.py | 5 | ||||
| -rw-r--r-- | test/engine/test_pool.py | 95 |
3 files changed, 117 insertions, 1 deletions
diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst index bf04aabca..2b8c5f3ad 100644 --- a/doc/build/changelog/changelog_09.rst +++ b/doc/build/changelog/changelog_09.rst @@ -12,6 +12,24 @@ :start-line: 5 .. changelog:: + :version: 0.9.11 + + .. change:: + :tags: bug, pool + :tickets: 3497 + :versions: 1.0.8 + + Fixed critical issue whereby the pool "checkout" event handler + may be called against a stale connection without the "connect" + event handler having been called, in the case where the pool + attempted to reconnect after being invalidated and failed; the stale + connection would remain present and would be used on a subsequent + attempt. This issue has a greater impact in the 1.0 series subsequent + to 1.0.2, as it also delivers a blanked-out ``.info`` dictionary to + the event handler; prior to 1.0.2 the ``.info`` dictionary is still + the previous one. + +.. changelog:: :version: 0.9.10 :released: July 22, 2015 diff --git a/lib/sqlalchemy/pool.py b/lib/sqlalchemy/pool.py index b38aefb3d..4dd954fc4 100644 --- a/lib/sqlalchemy/pool.py +++ b/lib/sqlalchemy/pool.py @@ -587,7 +587,12 @@ class _ConnectionRecord(object): if recycle: self.__close() self.info.clear() + + # ensure that if self.__connect() fails, + # we are not referring to the previous stale connection here + self.connection = None self.connection = self.__connect() + if self.__pool.dispatch.connect: self.__pool.dispatch.connect(self.connection, self) return self.connection diff --git a/test/engine/test_pool.py b/test/engine/test_pool.py index 451cb8b0e..8551e1fcb 100644 --- a/test/engine/test_pool.py +++ b/test/engine/test_pool.py @@ -8,8 +8,9 @@ from sqlalchemy.testing import eq_, assert_raises, is_not_, is_ from sqlalchemy.testing.engines import testing_engine from sqlalchemy.testing import fixtures import random -from sqlalchemy.testing.mock import Mock, call, patch +from sqlalchemy.testing.mock import Mock, call, patch, ANY import weakref +import collections join_timeout = 10 @@ -1480,6 +1481,98 @@ class QueuePoolTest(PoolTestBase): time.sleep(1.5) self._assert_cleanup_on_pooled_reconnect(dbapi, p) + def test_connect_handler_not_called_for_recycled(self): + """test [ticket:3497]""" + + dbapi, p = self._queuepool_dbapi_fixture( + pool_size=2, max_overflow=2) + + canary = Mock() + + c1 = p.connect() + c2 = p.connect() + + c1.close() + c2.close() + + dbapi.shutdown(True) + + bad = p.connect() + p._invalidate(bad) + bad.close() + assert p._invalidate_time + + event.listen(p, "connect", canary.connect) + event.listen(p, "checkout", canary.checkout) + + assert_raises( + Exception, + p.connect + ) + + p._pool.queue = collections.deque( + [ + c for c in p._pool.queue + if c.connection is not None + ] + ) + + dbapi.shutdown(False) + c = p.connect() + c.close() + + eq_( + canary.mock_calls, + [ + call.connect(ANY, ANY), + call.checkout(ANY, ANY, ANY) + ] + ) + + def test_connect_checkout_handler_always_gets_info(self): + """test [ticket:3497]""" + + dbapi, p = self._queuepool_dbapi_fixture( + pool_size=2, max_overflow=2) + + c1 = p.connect() + c2 = p.connect() + + c1.close() + c2.close() + + dbapi.shutdown(True) + + bad = p.connect() + p._invalidate(bad) + bad.close() + assert p._invalidate_time + + @event.listens_for(p, "connect") + def connect(conn, conn_rec): + conn_rec.info['x'] = True + + @event.listens_for(p, "checkout") + def checkout(conn, conn_rec, conn_f): + assert 'x' in conn_rec.info + + assert_raises( + Exception, + p.connect + ) + + p._pool.queue = collections.deque( + [ + c for c in p._pool.queue + if c.connection is not None + ] + ) + + dbapi.shutdown(False) + c = p.connect() + c.close() + + def test_error_on_pooled_reconnect_cleanup_wcheckout_event(self): dbapi, p = self._queuepool_dbapi_fixture(pool_size=1, max_overflow=2) |