- Fixed some "double invalidate" situations were detected where

a connection invalidation could occur within an already critical section
like a connection.close(); ultimately, these conditions are caused
by the change in :ticket:`2907`, in that the "reset on return" feature
calls out to the Connection/Transaction in order to handle it, where
"disconnect detection" might be caught.  However, it's possible that
the more recent change in :ticket:`2985` made it more likely for this
to be seen as the "connection invalidate" operation is much quicker,
as the issue is more reproducible on 0.9.4 than 0.9.3.

Checks are now added within any section that
an invalidate might occur to halt further disallowed operations
on the invalidated connection.  This includes two fixes both at the
engine level and at the pool level.   While the issue was observed
with highly concurrent gevent cases, it could in theory occur in
any kind of scenario where a disconnect occurs within the connection
close operation.
fixes #3043
ref #2985
ref #2907

- add some defensive checks during an invalidate situation:
1. _ConnectionRecord.invalidate might be called twice within finalize_fairy
if the _reset() raises an invalidate condition, invalidates, raises and then
goes to invalidate the CR.  so check for this.
2. similarly within Conneciton, anytime we do handle_dbapi_error(), we might become invalidated.
so a following finally must check self.__invalid before dealing with the connection
any futher.

diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index cd8cb0ac16701f6db6a6f3ad1218c6bb3ea9c31a..14ef6a601ccf0a6b0b5245b9011b3472cc5a2721 100644 (file)
--- a/doc/build/changelog/changelog_09.rst
+++ b/doc/build/changelog/changelog_09.rst
@@ -14,6 +14,28 @@
 .. changelog::
     :version: 0.9.5
 
+    .. change::
+        :tags: bug, engine
+        :tickets: 3043
+
+        Fixed some "double invalidate" situations were detected where
+        a connection invalidation could occur within an already critical section
+        like a connection.close(); ultimately, these conditions are caused
+        by the change in :ticket:`2907`, in that the "reset on return" feature
+        calls out to the Connection/Transaction in order to handle it, where
+        "disconnect detection" might be caught.  However, it's possible that
+        the more recent change in :ticket:`2985` made it more likely for this
+        to be seen as the "connection invalidate" operation is much quicker,
+        as the issue is more reproducible on 0.9.4 than 0.9.3.
+
+        Checks are now added within any section that
+        an invalidate might occur to halt further disallowed operations
+        on the invalidated connection.  This includes two fixes both at the
+        engine level and at the pool level.   While the issue was observed
+        with highly concurrent gevent cases, it could in theory occur in
+        any kind of scenario where a disconnect occurs within the connection
+        close operation.
+
     .. change::
         :tags: feature, orm
         :tickets: 3029

diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py
index bb3b82eea01f477796825b04af96881934104b63..997991b300c7d0afc2eade17348d5f6ea7496143 100644 (file)
--- a/lib/sqlalchemy/engine/base.py
+++ b/lib/sqlalchemy/engine/base.py
@@ -507,7 +507,8 @@ class Connection(Connectable):
             except Exception as e:
                 self._handle_dbapi_exception(e, None, None, None, None)
             finally:
-                if self.connection._reset_agent is self.__transaction:
+                if not self.__invalid and \
+                        self.connection._reset_agent is self.__transaction:
                     self.connection._reset_agent = None
                 self.__transaction = None
         else:
@@ -524,7 +525,8 @@ class Connection(Connectable):
         except Exception as e:
             self._handle_dbapi_exception(e, None, None, None, None)
         finally:
-            if self.connection._reset_agent is self.__transaction:
+            if not self.__invalid and \
+                    self.connection._reset_agent is self.__transaction:
                 self.connection._reset_agent = None
             self.__transaction = None
 
@@ -637,7 +639,12 @@ class Connection(Connectable):
                 conn.close()
             if conn._reset_agent is self.__transaction:
                 conn._reset_agent = None
-            del self.__connection
+
+            # the close() process can end up invalidating us,
+            # as the pool will call our transaction as the "reset_agent"
+            # for rollback(), which can then cause an invalidation
+            if not self.__invalid:
+                del self.__connection
         self.__can_reconnect = False
         self.__transaction = None
 

diff --git a/lib/sqlalchemy/pool.py b/lib/sqlalchemy/pool.py
index 799443546fe45df366f6616d8d9a418e6489889b..4020311a00f9d522045a29f3c7fa4928113dac53 100644 (file)
--- a/lib/sqlalchemy/pool.py
+++ b/lib/sqlalchemy/pool.py
@@ -479,6 +479,9 @@ class _ConnectionRecord(object):
             :ref:`pool_connection_invalidation`
 
         """
+        # already invalidated
+        if self.connection is None:
+            return
         self.__pool.dispatch.invalidate(self.connection, self, e)
         if e is not None:
             self.__pool.logger.info(
@@ -557,6 +560,7 @@ def _finalize_fairy(connection, connection_record, pool, ref, echo, fairy=None):
             if not connection_record:
                 pool._close_connection(connection)
         except Exception as e:
+            pool.logger.error("Exception during reset or similar", exc_info=True)
             if connection_record:
                 connection_record.invalidate(e=e)
             if isinstance(e, (SystemExit, KeyboardInterrupt)):