diff --git a/core/lib/Drupal/Core/Database/Connection.php b/core/lib/Drupal/Core/Database/Connection.php index 9f98b00a2785323626a9d2f8f73e7de76ec6d597..a5fad548940465781c98948ef8e3b5c3cab7b128 100644 --- a/core/lib/Drupal/Core/Database/Connection.php +++ b/core/lib/Drupal/Core/Database/Connection.php @@ -292,6 +292,31 @@ public function __destruct() { $this->connection = NULL; } + /** + * Commits all the open transactions. + * + * @internal + * This method exists only to work around a bug caused by Drupal incorrectly + * relying on object destruction order to commit transactions. Xdebug 3.3.0 + * changes the order of object destruction when the develop mode is enabled. + */ + public function commitAll() { + $manager = $this->transactionManager(); + if ($manager && $manager->inTransaction() && method_exists($manager, 'commitAll')) { + $this->transactionManager()->commitAll(); + } + + // BC layer. + // @phpstan-ignore-next-line + if (!empty($this->transactionLayers)) { + // Make all transactions committable. + // @phpstan-ignore-next-line + $this->transactionLayers = array_fill_keys(array_keys($this->transactionLayers), FALSE); + // @phpstan-ignore-next-line + $this->popCommittableTransactions(); + } + } + /** * Returns the client-level database connection object. * diff --git a/core/lib/Drupal/Core/Database/Database.php b/core/lib/Drupal/Core/Database/Database.php index c76a23cafd768561ed3f600283184af48b84b00f..ee5d6597ecb959921efee2c6df74653fb1ca6ca3 100644 --- a/core/lib/Drupal/Core/Database/Database.php +++ b/core/lib/Drupal/Core/Database/Database.php @@ -486,10 +486,18 @@ public static function closeConnection($target = NULL, $key = NULL) { if (!isset($key)) { $key = self::$activeKey; } - if (isset($target)) { + if (isset($target) && isset(self::$connections[$key][$target])) { + if (self::$connections[$key][$target] instanceof Connection) { + self::$connections[$key][$target]->commitAll(); + } unset(self::$connections[$key][$target]); } - else { + elseif (isset(self::$connections[$key])) { + foreach (self::$connections[$key] as $connection) { + if ($connection instanceof Connection) { + $connection->commitAll(); + } + } unset(self::$connections[$key]); } // Force garbage collection to run. This ensures that client connection @@ -740,4 +748,45 @@ private static function isWithinModuleNamespace(string $namespace) { return ($first === 'Drupal' && strtolower($second) === $second); } + /** + * Calls commitAll() on all the open connections. + * + * If drupal_register_shutdown_function() exists the commit will occur during + * shutdown so that it occurs at the latest possible moment. + * + * @param bool $shutdown + * Internal param to denote that the method is being called by + * _drupal_shutdown_function(). + * + * @return void + * + * @internal + * This method exists only to work around a bug caused by Drupal incorrectly + * relying on object destruction order to commit transactions. Xdebug 3.3.0 + * changes the order of object destruction when the develop mode is enabled. + */ + public static function commitAllOnShutdown(bool $shutdown = FALSE): void { + static $registered = FALSE; + + if ($shutdown) { + foreach (self::$connections as $targets) { + foreach ($targets as $connection) { + if ($connection instanceof Connection) { + $connection->commitAll(); + } + } + } + return; + } + + if (!function_exists('drupal_register_shutdown_function')) { + return; + } + + if (!$registered) { + $registered = TRUE; + drupal_register_shutdown_function('\Drupal\Core\Database\Database::commitAllOnShutdown', TRUE); + } + } + } diff --git a/core/lib/Drupal/Core/Database/Transaction.php b/core/lib/Drupal/Core/Database/Transaction.php index 062dd177d09e6ee3a63af1bcb6acf4d1de46c153..2c1ea5e0458b97bac34e74a16f7a823128f67f02 100644 --- a/core/lib/Drupal/Core/Database/Transaction.php +++ b/core/lib/Drupal/Core/Database/Transaction.php @@ -57,6 +57,11 @@ public function __construct( $name = NULL, protected readonly string $id = '', ) { + // Transactions rely on objects being destroyed in order to be committed. + // PHP makes no guarantee about the order in which objects are destroyed so + // ensure all transactions are committed on shutdown. + Database::commitAllOnShutdown(); + if ($connection->transactionManager()) { $this->connection = $connection; $this->name = $name; diff --git a/core/lib/Drupal/Core/Database/Transaction/TransactionManagerBase.php b/core/lib/Drupal/Core/Database/Transaction/TransactionManagerBase.php index 7aa66fac0b40f2af50f889db727c79424b09a333..932946a82d43db886f075131548e6302595071dc 100644 --- a/core/lib/Drupal/Core/Database/Transaction/TransactionManagerBase.php +++ b/core/lib/Drupal/Core/Database/Transaction/TransactionManagerBase.php @@ -127,6 +127,20 @@ protected function stack(): array { return $this->stack; } + /** + * Commits the entire transaction stack. + * + * @internal + * This method exists only to work around a bug caused by Drupal incorrectly + * relying on object destruction order to commit transactions. Xdebug 3.3.0 + * changes the order of object destruction when the develop mode is enabled. + */ + public function commitAll(): void { + foreach (array_reverse($this->stack()) as $id => $item) { + $this->unpile($item->name, $id); + } + } + /** * Adds an item to the transaction stack. * @@ -253,7 +267,6 @@ public function unpile(string $name, string $id): void { // DDL statement breaking an active transaction). That should be listed in // $voidedItems, so we can remove it from there. if (!isset($this->stack()[$id]) || $this->stack()[$id]->name !== $name) { - assert(isset($this->voidedItems[$id]), "Transaction {$id}/{$name} is out of sequence. Active stack: " . $this->dumpStackItemsAsString()); unset($this->voidedItems[$id]); return; } diff --git a/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificTransactionTestBase.php b/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificTransactionTestBase.php index 83f13c24771e15988bd98ca1665efa266a768215..c3f7275f7f73b5cd2fec5d32c14f81fcd532aa11 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificTransactionTestBase.php +++ b/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificTransactionTestBase.php @@ -4,8 +4,10 @@ use Drupal\Core\Database\Database; use Drupal\Core\Database\Transaction; +use Drupal\Core\Database\Transaction\ClientConnectionTransactionState; use Drupal\Core\Database\Transaction\StackItem; use Drupal\Core\Database\Transaction\StackItemType; +use Drupal\Core\Database\Transaction\TransactionManagerBase; use Drupal\Core\Database\TransactionExplicitCommitNotAllowedException; use Drupal\Core\Database\TransactionNameNonUniqueException; use Drupal\Core\Database\TransactionOutOfOrderException; @@ -807,11 +809,24 @@ public function testTransactionManagerFailureOnPendingStackItems(): void { $testConnection = Database::getConnection('test_fail'); // Add a fake item to the stack. - $reflectionMethod = new \ReflectionMethod(get_class($testConnection->transactionManager()), 'addStackItem'); - $reflectionMethod->invoke($testConnection->transactionManager(), 'bar', new StackItem('qux', StackItemType::Savepoint)); + $manager = $testConnection->transactionManager(); + $reflectionMethod = new \ReflectionMethod($manager, 'addStackItem'); + $reflectionMethod->invoke($manager, 'bar', new StackItem('qux', StackItemType::Root)); + // Ensure transaction state can be determined during object destruction. + // This is necessary for the test to pass when xdebug.mode has the 'develop' + // option enabled. + $reflectionProperty = new \ReflectionProperty(TransactionManagerBase::class, 'connectionTransactionState'); + $reflectionProperty->setValue($manager, ClientConnectionTransactionState::Active); $this->expectException(\AssertionError::class); $this->expectExceptionMessageMatches("/^Transaction .stack was not empty\\. Active stack: bar\\\\qux/"); + // Ensure that __destruct() results in an assertion error. Note that this + // will normally be called by PHP during the object's destruction but Drupal + // will commit all transactions when a database is closed thereby making + // this impossible to test unless it is called directly. + $manager->__destruct(); + + // Clean up. unset($testConnection); Database::closeConnection('test_fail'); } @@ -854,6 +869,10 @@ public function testConnectionDeprecations(): void { $this->expectDeprecation('Drupal\\Core\\Database\\Connection::popCommittableTransactions() is deprecated in drupal:10.2.0 and is removed from drupal:11.0.0. Use TransactionManagerInterface methods instead. See https://www.drupal.org/node/3381002'); $this->expectDeprecation('Drupal\\Core\\Database\\Connection::doCommit() is deprecated in drupal:10.2.0 and is removed from drupal:11.0.0. Use TransactionManagerInterface methods instead. See https://www.drupal.org/node/3381002'); $this->connection->popTransaction('foo'); + + // Ensure there are no outstanding transactions left. This is necessary for + // the test to pass when xdebug.mode has the 'develop' option enabled. + $this->connection->commitAll(); } }