diff options
| -rw-r--r-- | doc/build/changelog/changelog_09.rst | 22 | ||||
| -rw-r--r-- | lib/sqlalchemy/engine/base.py | 13 | ||||
| -rw-r--r-- | lib/sqlalchemy/pool.py | 4 |
3 files changed, 36 insertions, 3 deletions
diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst index cd8cb0ac1..14ef6a601 100644 --- a/doc/build/changelog/changelog_09.rst +++ b/doc/build/changelog/changelog_09.rst @@ -15,6 +15,28 @@ :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 bb3b82eea..997991b30 100644 --- 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 799443546..4020311a0 100644 --- 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)): |