[FIX] core: when flush() crashes while exiting a savepoint

When exiting a flushing savepoint, the method flush() is invoked before
closing the savepoint (which releases it):

    def _close(self, rollback):
        if not rollback:
            self._cr.flush()
        super()._close(rollback)

If method flush() crashes at that point, the savepoint is not closed at
all (because of the exception) and is therefore not even rolled back.
The expected behavior is that the savepoint rollbacks and the exception
bubbles up.

closes odoo/odoo#126866

X-original-commit: 5f9b4f5c998388625de27fbd8fe0f7a0524c04fd
Signed-off-by: Rémy Voet (ryv) <ryv@odoo.com>
Signed-off-by: Raphael Collet <rco@odoo.com>

odoo/addons/test_testing_utilities/tests/test_methods.py

@@ -51,7 +51,7 @@ class TestBasic(common.TransactionCase):
         self.env.cr.execute("SHOW test_testing_utilities.a_flag")
         self.assertEqual(self.env.cr.fetchone(), ('',))
 
-    def test_assertRaises_error(self):
+    def test_assertRaises_error_at_setup(self):
         """Checks that an exception raised during the *setup* of assertRaises
         bubbles up correctly.
 
@@ -63,6 +63,19 @@ class TestBasic(common.TransactionCase):
             with self.assertRaises(CustomError):
                 raise NotImplementedError
 
+    def test_assertRaises_error_at_exit(self):
+        """Checks that a "correctly" executing assertRaises (where the expected
+        exception has been raised and caught) will properly rollback when the
+        error is raised by flush() while exiting the savepoint.
+        """
+        self.env.cr.execute("SET LOCAL test_testing_utilities.a_flag = ''")
+        with mock.patch.object(BaseCursor, 'flush', side_effect=[None, CustomError]):
+            with self.assertRaises(CustomError):
+                self.env.cr.execute("SET LOCAL test_testing_utilities.a_flag = 'yes'")
+
+        self.env.cr.execute("SHOW test_testing_utilities.a_flag")
+        self.assertEqual(self.env.cr.fetchone(), ('',))
+
     @mute_logger('odoo.sql_db')
     def test_assertRaises_clear_recovery(self):
         """Checks that the savepoint is correctly rolled back if an error occurs

odoo/sql_db.py

@@ -96,6 +96,7 @@ class Savepoint:
         self._cr.execute(SQL('RELEASE SAVEPOINT {}').format(self._name))
         self.closed = True
 
+
 class _FlushingSavepoint(Savepoint):
     def __init__(self, cr):
         cr.flush()
@@ -106,9 +107,15 @@ class _FlushingSavepoint(Savepoint):
         super().rollback()
 
     def _close(self, rollback):
-        if not rollback:
-            self._cr.flush()
-        super()._close(rollback)
+        try:
+            if not rollback:
+                self._cr.flush()
+        except Exception:
+            rollback = True
+            raise
+        finally:
+            super()._close(rollback)
+
 
 class BaseCursor:
     """ Base class for cursors that manage pre/post commit hooks. """