Commit 2ee59bef authored by catch's avatar catch
Browse files

Issue #3128616 by alexpott, daffie, mondrake, xjm, Charlie ChX Negyesi:...

Issue #3128616 by alexpott, daffie, mondrake, xjm, Charlie ChX Negyesi: Replace \Drupal\Core\Database\Connection::destroy() with a proper destructor
parent 4df1ca4f
......@@ -258,15 +258,39 @@ public static function open(array &$connection_options = []) {}
* variables. In case of PDO database connection objects, PHP only closes the
* connection when the PDO object is destructed, so any references to this
* object may cause the number of maximum allowed connections to be exceeded.
*
* @deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Move custom
* database destruction logic to __destruct().
*
* @see https://www.drupal.org/node/3142866
*/
public function destroy() {
// Destroy all references to this connection by setting them to NULL.
// The Statement class attribute only accepts a new value that presents a
// proper callable, so we reset it to PDOStatement.
if (!empty($this->statementClass)) {
$this->connection->setAttribute(\PDO::ATTR_STATEMENT_CLASS, ['PDOStatement', []]);
$backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2);
if ($backtrace[1]['class'] !== self::class && $backtrace[1]['function'] !== '__destruct') {
@trigger_error(__METHOD__ . '() is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Move custom database destruction logic to __destruct(). See https://www.drupal.org/node/3142866', E_USER_DEPRECATED);
// Destroy all references to this connection by setting them to NULL.
// The Statement class attribute only accepts a new value that presents a
// proper callable, so we reset it to PDOStatement.
if (!empty($this->statementClass)) {
$this->connection->setAttribute(\PDO::ATTR_STATEMENT_CLASS, ['PDOStatement', []]);
}
$this->schema = NULL;
}
$this->schema = NULL;
}
/**
* Ensures that the PDO connection can be garbage collected.
*/
public function __destruct() {
// Call the ::destroy method to provide a BC layer.
// @todo https://www.drupal.org/project/drupal/issues/3153864 Remove this
// call in Drupal 10 as the logic in the destroy() method is no longer
// required now we implement a proper destructor.
$this->destroy();
// Ensure that the circular reference caused by Connection::__construct()
// using $this in the call to set the statement class can be garbage
// collected.
$this->connection = NULL;
}
/**
......
......@@ -405,26 +405,15 @@ public static function closeConnection($target = NULL, $key = NULL) {
if (!isset($key)) {
$key = self::$activeKey;
}
// To close a connection, it needs to be set to NULL and removed from the
// static variable. In all cases, closeConnection() might be called for a
// connection that was not opened yet, in which case the key is not defined
// yet and we just ensure that the connection key is undefined.
if (isset($target)) {
if (isset(self::$connections[$key][$target])) {
self::$connections[$key][$target]->destroy();
self::$connections[$key][$target] = NULL;
}
unset(self::$connections[$key][$target]);
}
else {
if (isset(self::$connections[$key])) {
foreach (self::$connections[$key] as $target => $connection) {
self::$connections[$key][$target]->destroy();
self::$connections[$key][$target] = NULL;
}
}
unset(self::$connections[$key]);
}
// Force garbage collection to run. This ensures that PDO connection objects
// and destroyed and results in the connections being closed.
gc_collect_cycles();
}
/**
......
......@@ -206,6 +206,7 @@ public function __destruct() {
if ($this->needsCleanup) {
$this->nextIdDelete();
}
parent::__destruct();
}
public function queryRange($query, $from, $count, array $args = [], array $options = []) {
......
......@@ -199,6 +199,7 @@ public function __destruct() {
}
}
}
parent::__destruct();
}
/**
......
......@@ -2,6 +2,8 @@
namespace Drupal\KernelTests\Core\Database;
use Drupal\Core\Database\Database;
/**
* Tests the sequences API.
*
......@@ -38,4 +40,37 @@ public function testDbNextId() {
$this->assertEqual($result, 1001, 'Sequence provides a larger number than the existing ID.');
}
/**
* Tests that sequences table clear up works when a connection is closed.
*
* @see \Drupal\Core\Database\Driver\mysql\Connection::__destruct()
*/
public function testDbNextIdClosedConnection() {
// Only run this test for the 'mysql' driver.
$driver = $this->connection->driver();
if ($driver !== 'mysql') {
$this->markTestSkipped("MySql tests can not run for driver '$driver'.");
}
// Create an additional connection to test closing the connection.
$connection_info = Database::getConnectionInfo();
Database::addConnectionInfo('default', 'next_id', $connection_info['default']);
// Get a few IDs to ensure there the clean up needs to run and there is more
// than one row.
Database::getConnection('next_id')->nextId();
Database::getConnection('next_id')->nextId();
// At this point the sequences table should contain unnecessary rows.
$count = $this->connection->select('sequences')->countQuery()->execute()->fetchField();
$this->assertGreaterThan(1, $count);
// Close the connection.
Database::closeConnection('next_id');
// Test that \Drupal\Core\Database\Driver\mysql\Connection::__destruct()
// successfully trims the sequences table if the connection is closed.
$count = $this->connection->select('sequences')->countQuery()->execute()->fetchField();
$this->assertEquals(1, $count);
}
}
......@@ -6,6 +6,7 @@
use Drupal\Core\Database\Statement;
use Drupal\Tests\Core\Database\Stub\StubConnection;
use Drupal\Tests\Core\Database\Stub\StubPDO;
use Drupal\Tests\Core\Database\Stub\Driver;
use Drupal\Tests\UnitTestCase;
/**
......@@ -320,6 +321,9 @@ public function testSchema($expected, $driver, $namespace) {
/**
* Test Connection::destroy().
*
* @group legacy
* @expectedDeprecation Drupal\Core\Database\Connection::destroy() is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Move custom database destruction logic to __destruct(). See https://www.drupal.org/node/3142866
*/
public function testDestroy() {
$mock_pdo = $this->createMock('Drupal\Tests\Core\Database\Stub\StubPDO');
......@@ -337,6 +341,29 @@ public function testDestroy() {
$this->assertNull($reflected_schema->getValue($connection));
}
/**
* Test Connection::__destruct().
*
* @group legacy
* @expectedDeprecation Drupal\Core\Database\Connection::destroy() is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Move custom database destruction logic to __destruct(). See https://www.drupal.org/node/3142866
*/
public function testDestructBcLayer() {
$mock_pdo = $this->createMock(StubPDO::class);
$fake_connection = new class($mock_pdo, ['namespace' => Driver::class]) extends StubConnection {
public function destroy() {
parent::destroy();
}
};
// Destroy the object which will result in the Connection::__destruct()
// calling Connection::destroy() and a deprecation error being triggered.
// @see \Drupal\KernelTests\Core\Database\ConnectionUnitTest for tests that
// connection object destruction does not trigger deprecations unless
// Connection::destroy() is overridden.
$fake_connection = NULL;
}
/**
* Dataprovider for testMakeComments().
*
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment