Closes #3440043

• /core/modules/mysql/src/Driver/Database/mysql/TransactionManager.php
• /core/tests/Drupal/KernelTests/Core/Database/DriverSpecificTransactionTestBase.php

Closes #3440043

core/lib/Drupal/Core/Database/Transaction/TransactionManagerBase.php

@@ -328,9 +328,11 @@ public function rollback(string $name, string $id): void {
         // If this was the root Drupal transaction, we can rollback the client
         // transaction. The transaction is closed.
         $this->processRootRollback();
-        // The Transaction object remains open, and when it will get destructed
-        // no commit should happen. Void the stack item.
-        $this->voidStackItem($id);
+        if ($this->getConnectionTransactionState() === ClientConnectionTransactionState::RolledBack) {
+          // The Transaction object remains open, and when it will get destructed
+          // no commit should happen. Void the stack item.
+          $this->voidStackItem($id);
+        }
       }
       else {
         // The stack got corrupted.

[mondrake]

BTW we discover and fix a bug here... ::processRootRollback() above already voids the item, so calling ::voidStackItem() on it fails, since it's removed from the stack.

The fact that nobody complained (this is a missing offset error just after the warning) probably tells a lot about whether this code path occurs in real world...

In our testing ATM, we do not get to ::voidStackItem() because in testTransactionWithDdlStatement() the warning is transformed into an exception (in PHPUnit 9), and then caught and never returning to code execution.

[mondrake]

By removing the trigger_error for PHPUnit 10 we could see the bug and fix here.