Issue #3405976 by alexpott, jrglasgow, fago, catch, mondrake, solideogloria,... (b09cf3ba) · Commits · project / drupal

core/lib/Drupal/Core/Database/Connection.php

+25 −0

Original line number	Diff line number	Diff line
		@@ -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.
		*

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

+51 −2

Original line number	Diff line number	Diff line
		@@ -493,10 +493,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
		@@ -747,4 +755,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);
		}
		}

		}

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

+5 −0

Original line number	Diff line number	Diff line
		@@ -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;

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

+14 −1

Original line number	Diff line number	Diff line
		@@ -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;
		}

core/tests/Drupal/KernelTests/Core/Database/DriverSpecificTransactionTestBase.php

+21 −2

Original line number	Diff line number	Diff line
		@@ -3,8 +3,10 @@
		namespace Drupal\KernelTests\Core\Database;

		use Drupal\Core\Database\Database;
		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;
		@@ -885,11 +887,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 with calling it directly.
		$manager->__destruct();

		// Clean up.
		unset($testConnection);
		Database::closeConnection('test_fail');
		}
		@@ -932,6 +947,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();
		}

		}