diff --git a/core/lib/Drupal/Core/Database/Transaction/TransactionManagerBase.php b/core/lib/Drupal/Core/Database/Transaction/TransactionManagerBase.php index c9b2fc6135f..4e1fc8352fa 100644 --- a/core/lib/Drupal/Core/Database/Transaction/TransactionManagerBase.php +++ b/core/lib/Drupal/Core/Database/Transaction/TransactionManagerBase.php @@ -23,6 +23,26 @@ use Drupal\Core\Database\TransactionOutOfOrderException; */ abstract class TransactionManagerBase implements TransactionManagerInterface { + /** + * The ID of the root Transaction object. + * + * The unique identifier of the first 'root' transaction object created, when + * the stack is empty. + * + * Normally, during the transaction stack lifecycle only one 'root' + * Transaction object is processed. Any post transaction callbacks are only + * processed during its destruction. However, there are cases when there + * could be multiple 'root' transaction objects in the stack. For example: a + * 'root' transaction object is opened, then a DDL statement is executed in a + * database that does not support transactional DDL, and because of that, + * another 'root' is opened before the original one is closed. + * + * Keeping track of the first 'root' created allows us to process the post + * transaction callbacks only during its destruction and not during + * destruction of another one. + */ + private ?string $rootId = NULL; + /** * The stack of Drupal transactions currently active. * @@ -234,11 +254,19 @@ abstract class TransactionManagerBase implements TransactionManagerInterface { throw new TransactionNameNonUniqueException("A transaction named {$name} is already in use. Active stack: " . $this->dumpStackItemsAsString()); } + // Define a unique ID for the transaction. + $id = uniqid('', TRUE); + // Do the client-level processing. if ($this->stackDepth() === 0) { $this->beginClientTransaction(); $type = StackItemType::Root; $this->setConnectionTransactionState(ClientConnectionTransactionState::Active); + // Only set ::rootId if there's not one set already, which may happen in + // case of broken transactions. + if ($this->rootId === NULL) { + $this->rootId = $id; + } } else { // If we're already in a Drupal transaction then we want to create a @@ -248,9 +276,6 @@ abstract class TransactionManagerBase implements TransactionManagerInterface { $type = StackItemType::Savepoint; } - // Define an unique id for the transaction. - $id = uniqid('', TRUE); - // Add an item on the stack, increasing its depth. $this->addStackItem($id, new StackItem($name, $type)); @@ -262,6 +287,18 @@ abstract class TransactionManagerBase implements TransactionManagerInterface { * {@inheritdoc} */ public function unpile(string $name, string $id): void { + // If this is a 'root' transaction, and it is voided (that is, no longer in + // the stack), then the transaction on the database is no longer active. An + // action such as a rollback, or a DDL statement, was executed that + // terminated the database transaction. So, we can process the post + // transaction callbacks. + if (!isset($this->stack()[$id]) && isset($this->voidedItems[$id]) && $this->rootId === $id) { + $this->processPostTransactionCallbacks(); + $this->rootId = NULL; + unset($this->voidedItems[$id]); + return; + } + // If the $id does not correspond to the one in the stack for that $name, // we are facing an orphaned Transaction object (for example in case of a // DDL statement breaking an active transaction). That should be listed in @@ -289,6 +326,10 @@ abstract class TransactionManagerBase implements TransactionManagerInterface { // If this was the root Drupal transaction, we can commit the client // transaction. $this->processRootCommit(); + if ($this->rootId === $id) { + $this->processPostTransactionCallbacks(); + $this->rootId = NULL; + } } else { // The stack got corrupted. @@ -405,7 +446,6 @@ abstract class TransactionManagerBase implements TransactionManagerInterface { * Processes the root transaction rollback. */ protected function processRootRollback(): void { - $this->processPostTransactionCallbacks(); $this->rollbackClientTransaction(); } @@ -417,7 +457,6 @@ abstract class TransactionManagerBase implements TransactionManagerInterface { */ protected function processRootCommit(): void { $clientCommit = $this->commitClientTransaction(); - $this->processPostTransactionCallbacks(); if (!$clientCommit) { throw new TransactionCommitFailedException(); } @@ -519,7 +558,6 @@ abstract class TransactionManagerBase implements TransactionManagerInterface { $this->voidStackItem((string) $i); } $this->setConnectionTransactionState(ClientConnectionTransactionState::Voided); - $this->processPostTransactionCallbacks(); } } diff --git a/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificTransactionTestBase.php b/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificTransactionTestBase.php index 42f9ea03902..3911419ca77 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificTransactionTestBase.php +++ b/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificTransactionTestBase.php @@ -776,18 +776,60 @@ class DriverSpecificTransactionTestBase extends DriverSpecificDatabaseTestBase { /** * Tests post-transaction callback executes after transaction rollback. */ - public function testRootTransactionEndCallbackCalledOnRollback(): void { + public function testRootTransactionEndCallbackCalledAfterRollbackAndDestruction(): void { $transaction = $this->createRootTransaction('', FALSE); $this->connection->transactionManager()->addPostTransactionCallback([$this, 'rootTransactionCallback']); $this->insertRow('row'); $this->assertNull($this->postTransactionCallbackAction); + + // Callbacks are processed only when destructing the transaction. + // Executing a rollback is not sufficient by itself. $transaction->rollBack(); - $this->assertSame('rtcRollback', $this->postTransactionCallbackAction); - unset($transaction); - $this->assertRowAbsent('row'); - // The row insert should be missing since the client rollback occurs after - // the processing of the callbacks. + $this->assertNull($this->postTransactionCallbackAction); + $this->assertRowAbsent('rtcCommit'); $this->assertRowAbsent('rtcRollback'); + $this->assertRowAbsent('row'); + + // Destruct the transaction. + unset($transaction); + + // The post-transaction callback should now have inserted a 'rtcRollback' + // row. + $this->assertSame('rtcRollback', $this->postTransactionCallbackAction); + $this->assertRowAbsent('rtcCommit'); + $this->assertRowPresent('rtcRollback'); + $this->assertRowAbsent('row'); + } + + /** + * Tests post-transaction callback executes after a DDL statement. + */ + public function testRootTransactionEndCallbackCalledAfterDdlAndDestruction(): void { + $transaction = $this->createRootTransaction('', FALSE); + $this->connection->transactionManager()->addPostTransactionCallback([$this, 'rootTransactionCallback']); + $this->insertRow('row'); + $this->assertNull($this->postTransactionCallbackAction); + + // Callbacks are processed only when destructing the transaction. + // Executing a DDL statement is not sufficient itself. + // We cannot use truncate here, since it has protective code to fall back + // to a transactional delete when in transaction. We drop an unrelated + // table instead. + $this->connection->schema()->dropTable('test_people'); + $this->assertNull($this->postTransactionCallbackAction); + $this->assertRowAbsent('rtcCommit'); + $this->assertRowAbsent('rtcRollback'); + $this->assertRowPresent('row'); + + // Destruct the transaction. + unset($transaction); + + // The post-transaction callback should now have inserted a 'rtcCommit' + // row. + $this->assertSame('rtcCommit', $this->postTransactionCallbackAction); + $this->assertRowPresent('rtcCommit'); + $this->assertRowAbsent('rtcRollback'); + $this->assertRowPresent('row'); } /**