Warn when transaction context manager ends on inactive transaction

if .rollback() or .commit() is called inside the transaction
context manager, the transaction object is deactivated.
the context manager continues but will not be able to correctly
fulfill it's closing state.  Ensure a warning is emitted when
this happens.

Change-Id: I8fc3a73f7c21575dda5bcbd6fb74ddb679771630

diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py
index 3e02a29fec42626c7e30e095b34ac443e5db919b..81c0c9f580c5fcf4af2bdbbf4b316892a9e63b40 100644 (file)
--- a/lib/sqlalchemy/engine/base.py
+++ b/lib/sqlalchemy/engine/base.py
@@ -2095,6 +2095,9 @@ class RootTransaction(Transaction):
             ):
                 self.connection._dbapi_connection._reset_agent = None
 
+        elif self.connection._transaction is not self:
+            util.warn("transaction already deassociated from connection")
+
         # we have tests that want to make sure the pool handles this
         # correctly.  TODO: how to disable internal assertions cleanly?
         # else:
@@ -2133,7 +2136,7 @@ class RootTransaction(Transaction):
     def _connection_commit_impl(self):
         self.connection._commit_impl()
 
-    def _close_impl(self):
+    def _close_impl(self, try_deactivate=False):
         try:
             if self.is_active:
                 self._connection_rollback_impl()
@@ -2141,7 +2144,7 @@ class RootTransaction(Transaction):
             if self.connection._nested_transaction:
                 self.connection._nested_transaction._cancel()
         finally:
-            if self.is_active:
+            if self.is_active or try_deactivate:
                 self._deactivate_from_connection()
             if self.connection._transaction is self:
                 self.connection._transaction = None
@@ -2153,7 +2156,7 @@ class RootTransaction(Transaction):
         self._close_impl()
 
     def _do_rollback(self):
-        self._close_impl()
+        self._close_impl(try_deactivate=True)
 
     def _do_commit(self):
         if self.is_active:

diff --git a/lib/sqlalchemy/testing/fixtures.py b/lib/sqlalchemy/testing/fixtures.py
index 041daf35e1d8f2e03f9214f50040e53d9478660a..2cc34448d517e1af9e3a94774369d981bdbd30f2 100644 (file)
--- a/lib/sqlalchemy/testing/fixtures.py
+++ b/lib/sqlalchemy/testing/fixtures.py
@@ -66,7 +66,8 @@ class TestBase(object):
         try:
             yield conn
         finally:
-            trans.rollback()
+            if trans.is_active:
+                trans.rollback()
             conn.close()
 
     # propose a replacement for @testing.provide_metadata.

diff --git a/test/engine/test_transaction.py b/test/engine/test_transaction.py
index 164604cd650f458dad88060708bb015531a66d30..b82d143f91a2891a485422ff77672bf7e2c3f947 100644 (file)
--- a/test/engine/test_transaction.py
+++ b/test/engine/test_transaction.py
@@ -320,6 +320,22 @@ class TransactionTest(fixtures.TestBase):
             1,
         )
 
+    def test_deactivated_warning_ctxmanager(self, local_connection):
+        with expect_warnings(
+            "transaction already deassociated from connection"
+        ):
+            with local_connection.begin() as trans:
+                trans.rollback()
+
+    @testing.requires.savepoints
+    def test_deactivated_savepoint_warning_ctxmanager(self, local_connection):
+        with expect_warnings(
+            "nested transaction already deassociated from connection"
+        ):
+            with local_connection.begin():
+                with local_connection.begin_nested() as savepoint:
+                    savepoint.rollback()
+
     def test_commit_fails_flat(self, local_connection):
         connection = local_connection
 
@@ -355,7 +371,10 @@ class TransactionTest(fixtures.TestBase):
 
         t1 = transaction[0]
         assert not t1.is_active
-        t1.rollback()  # no error
+        with expect_warnings(
+            "transaction already deassociated from connection"
+        ):
+            t1.rollback()  # no error
 
     @testing.requires.savepoints_w_release
     def test_savepoint_rollback_fails_flat(self, local_connection):