connectionrec recycle can create situation where checkout handler is called w/o connect handler being successful

[Mike Bayer]

diff --git a/lib/sqlalchemy/pool.py b/lib/sqlalchemy/pool.py
index b38aefb..9af956b 100644
--- a/lib/sqlalchemy/pool.py
+++ b/lib/sqlalchemy/pool.py
@@ -453,6 +453,7 @@ class _ConnectionRecord(object):
             for_modify(pool.dispatch).\
             exec_once(self.connection, self)
         pool.dispatch.connect(self.connection, self)
+        self.info['x'] = 1

     connection = None
     """A reference to the actual DBAPI connection being tracked.
@@ -563,6 +564,8 @@ class _ConnectionRecord(object):
             self.connection = self.__connect()
             if self.__pool.dispatch.connect:
                 self.__pool.dispatch.connect(self.connection, self)
+            self.info['x'] = 1
+
         elif self.__pool._recycle > -1 and \
                 time.time() - self.starttime > self.__pool._recycle:
             self.__pool.logger.info(
@@ -587,9 +590,17 @@ class _ConnectionRecord(object):
         if recycle:
             self.__close()
             self.info.clear()
+
+            # this is the fix; ensure self.connection is None first
+            # self.connection = None
+
+            # here, if __connect() fails, self.connection
+            # is still referring to the old connection, but info is empty
+            # and we won't attempt a __connect() next time we are here
             self.connection = self.__connect()
             if self.__pool.dispatch.connect:
                 self.__pool.dispatch.connect(self.connection, self)
+            self.info['x'] = 1
         return self.connection

     def __close(self):
@@ -718,12 +729,14 @@ class _ConnectionFairy(object):
         fairy._counter += 1

         if not pool.dispatch.checkout or fairy._counter != 1:
+            assert 'x' in fairy.info
             return fairy

         # Pool listeners can trigger a reconnection on checkout
         attempts = 2
         while attempts > 0:
             try:
+                assert fairy.info['x']
                 pool.dispatch.checkout(fairy.connection,
                                        fairy._connection_record,
                                        fairy)

then run:

py.test test/engine/test_pool.py  -k test_error_on_pooled_reconnect_cleanup_recycle

this warrants an immediate 1.0.8 IMO, I would like to try to produce an oslo.db-level reproduction case though

[Mike Bayer]

so what's interesting to note here is that while the issue probably exists in 0.9 as well, we have a different system of resetting the ".info" there where we do it after the connect attempt. We've moved that up to suit the HAAlchemy use case in 1.0. but this is an 0.9 issue regardless so when we test we should find a test that looks at it from the perspective of the connection events themselves and not necessarily .info.

[Mike Bayer]

• 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. fixes #3497

→ <<cset 4096ad0f0980>>

[Mike Bayer]

• 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. fixes #3497

(cherry picked from commit 4096ad0f0980f6940be57aaee85791df8f975cd7)

→ <<cset 0aee8eb0015b>>