Drupal
/
drupal
mirror of https://git.drupalcode.org/project/drupal.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Issue #843114 by sun, quicksketch, Berdir | c960657: Fixed DatabaseConnection::__construct() and DatabaseConnection_mysql()::__construct() leaks $this (Too many connections).

merge-requests/26/head
David Rothstein 2012-12-08 18:31:33 -05:00
parent ef10c14b7f
commit c7e45d937d
4 changed files with 266 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
CHANGELOG.txt
View File

@ -1,9 +1,11 @@
Drupal 7.18, xxxx-xx-xx (development version)
-----------------------
- Fixed a bug which caused the database API to not properly close database
connections.
- Added link to the URL for running cron from outside the site to the Cron
settings page (UI change).
- Fixed bug which prevented image styles from being reverted on PHP 5.4.
- Fixed a bug which prevented image styles from being reverted on PHP 5.4.
- Made the default .htaccess rules protocol sensitive to improve security for
sites which use HTTPS and redirect between "www" and non-"www" versions of
the page.

41
includes/database/database.inc
View File

@ -194,7 +194,7 @@ abstract class DatabaseConnection extends PDO {
/**
* The key representing this connection.
*
*
* The key is a unique string which identifies a database connection. A
* connection can be a single server or a cluster of master and slaves (use
* target to pick between master and slave).
@ -303,12 +303,28 @@ abstract class DatabaseConnection extends PDO {
// Call PDO::__construct and PDO::setAttribute.
parent::__construct($dsn, $username, $password, $driver_options);
// Set a specific PDOStatement class if the driver requires that.
// Set a Statement class, unless the driver opted out.
if (!empty($this->statementClass)) {
$this->setAttribute(PDO::ATTR_STATEMENT_CLASS, array($this->statementClass, array($this)));
}
}
/**
* Destroys this Connection object.
*
* PHP does not destruct an object if it is still referenced in other
* 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.
*/
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.
$this->setAttribute(PDO::ATTR_STATEMENT_CLASS, array('PDOStatement', array()));
$this->schema = NULL;
}
/**
* Returns the default query options for any given query.
*
@ -1627,8 +1643,8 @@ abstract class Database {
*/
final public static function removeConnection($key) {
if (isset(self::$databaseInfo[$key])) {
self::closeConnection(NULL, $key);
unset(self::$databaseInfo[$key]);
unset(self::$connections[$key]);
return TRUE;
}
else {
@ -1694,11 +1710,24 @@ abstract class Database {
if (!isset($key)) {
$key = self::$activeKey;
}
// To close the connection, we need to unset the static variable.
// 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]);
}
}
@ -1852,8 +1881,8 @@ class DatabaseTransaction {
*/
protected $name;
public function __construct(DatabaseConnection &$connection, $name = NULL) {
$this->connection = &$connection;
public function __construct(DatabaseConnection $connection, $name = NULL) {
$this->connection = $connection;
// If there is no transaction depth, then no transaction has started. Name
// the transaction 'drupal_transaction'.
if (!$depth = $connection->transactionDepth()) {

17
includes/database/mysql/database.inc
View File

@ -13,11 +13,11 @@
class DatabaseConnection_mysql extends DatabaseConnection {
/**
* Flag to indicate if we have registered the nextID cleanup function.
* Flag to indicate if the cleanup function in __destruct() should run.
*
* @var boolean
*/
protected $shutdownRegistered = FALSE;
protected $needsCleanup = FALSE;
public function __construct(array $connection_options = array()) {
// This driver defaults to transaction support, except if explicitly passed FALSE.
@ -78,6 +78,12 @@ class DatabaseConnection_mysql extends DatabaseConnection {
$this->exec(implode('; ', $connection_options['init_commands']));
}
public function __destruct() {
if ($this->needsCleanup) {
$this->nextIdDelete();
}
}
public function queryRange($query, $from, $count, array $args = array(), array $options = array()) {
return $this->query($query . ' LIMIT ' . (int) $from . ', ' . (int) $count, $args, $options);
}
@ -115,12 +121,7 @@ class DatabaseConnection_mysql extends DatabaseConnection {
$this->query('INSERT INTO {sequences} (value) VALUES (:value) ON DUPLICATE KEY UPDATE value = value', array(':value' => $existing_id));
$new_id = $this->query('INSERT INTO {sequences} () VALUES ()', array(), array('return' => Database::RETURN_INSERT_ID));
}
if (!$this->shutdownRegistered) {
// Use register_shutdown_function() here to keep the database system
// independent of Drupal.
register_shutdown_function(array($this, 'nextIdDelete'));
$shutdownRegistered = TRUE;
}
$this->needsCleanup = TRUE;
return $new_id;
}

219
modules/simpletest/tests/database_test.test
View File

@ -3815,3 +3815,222 @@ class DatabaseEmptyStatementTestCase extends DrupalWebTestCase {
$this->assertEqual($result->fetchAll(), array(), t('Empty array returned from empty result set.'));
}
}
/**
* Tests management of database connections.
*/
class ConnectionUnitTest extends DrupalUnitTestCase {
protected $key;
protected $target;
protected $monitor;
protected $originalCount;
public static function getInfo() {
return array(
'name' => 'Connection unit tests',
'description' => 'Tests management of database connections.',
'group' => 'Database',
);
}
function setUp() {
parent::setUp();
$this->key = 'default';
$this->originalTarget = 'default';
$this->target = 'DatabaseConnectionUnitTest';
// Determine whether the database driver is MySQL. If it is not, the test
// methods will not be executed.
// @todo Make this test driver-agnostic, or find a proper way to skip it.
// @see http://drupal.org/node/1273478
$connection_info = Database::getConnectionInfo('default');
$this->skipTest = (bool) $connection_info['default']['driver'] != 'mysql';
if ($this->skipTest) {
// Insert an assertion to prevent Simpletest from interpreting the test
// as failure.
$this->pass('This test is only compatible with MySQL.');
}
// Create an additional connection to monitor the connections being opened
// and closed in this test.
// @see TestBase::changeDatabasePrefix()
$connection_info = Database::getConnectionInfo('default');
Database::addConnectionInfo('default', 'monitor', $connection_info['default']);
global $databases;
$databases['default']['monitor'] = $connection_info['default'];
$this->monitor = Database::getConnection('monitor');
}
/**
* Adds a new database connection info to Database.
*/
protected function addConnection() {
// Add a new target to the connection, by cloning the current connection.
$connection_info = Database::getConnectionInfo($this->key);
Database::addConnectionInfo($this->key, $this->target, $connection_info[$this->originalTarget]);
// Verify that the new target exists.
$info = Database::getConnectionInfo($this->key);
// Note: Custom assertion message to not expose database credentials.
$this->assertIdentical($info[$this->target], $connection_info[$this->key], 'New connection info found.');
}
/**
* Returns the connection ID of the current test connection.
*
* @return integer
*/
protected function getConnectionID() {
return (int) Database::getConnection($this->target, $this->key)->query('SELECT CONNECTION_ID()')->fetchField();
}
/**
* Asserts that a connection ID exists.
*
* @param integer $id
* The connection ID to verify.
*/
protected function assertConnection($id) {
$list = $this->monitor->query('SHOW PROCESSLIST')->fetchAllKeyed(0, 0);
return $this->assertTrue(isset($list[$id]), format_string('Connection ID @id found.', array('@id' => $id)));
}
/**
* Asserts that a connection ID does not exist.
*
* @param integer $id
* The connection ID to verify.
*/
protected function assertNoConnection($id) {
$list = $this->monitor->query('SHOW PROCESSLIST')->fetchAllKeyed(0, 0);
return $this->assertFalse(isset($list[$id]), format_string('Connection ID @id not found.', array('@id' => $id)));
}
/**
* Tests Database::closeConnection() without query.
*
* @todo getConnectionID() executes a query.
*/
function testOpenClose() {
if ($this->skipTest) {
return;
}
// Add and open a new connection.
$this->addConnection();
$id = $this->getConnectionID();
Database::getConnection($this->target, $this->key);
// Verify that there is a new connection.
$this->assertConnection($id);
// Close the connection.
Database::closeConnection($this->target, $this->key);
// Wait 20ms to give the database engine sufficient time to react.
usleep(20000);
// Verify that we are back to the original connection count.
$this->assertNoConnection($id);
}
/**
* Tests Database::closeConnection() with a query.
*/
function testOpenQueryClose() {
if ($this->skipTest) {
return;
}
// Add and open a new connection.
$this->addConnection();
$id = $this->getConnectionID();
Database::getConnection($this->target, $this->key);
// Verify that there is a new connection.
$this->assertConnection($id);
// Execute a query.
Database::getConnection($this->target, $this->key)->query('SHOW TABLES');
// Close the connection.
Database::closeConnection($this->target, $this->key);
// Wait 20ms to give the database engine sufficient time to react.
usleep(20000);
// Verify that we are back to the original connection count.
$this->assertNoConnection($id);
}
/**
* Tests Database::closeConnection() with a query and custom prefetch method.
*/
function testOpenQueryPrefetchClose() {
if ($this->skipTest) {
return;
}
// Add and open a new connection.
$this->addConnection();
$id = $this->getConnectionID();
Database::getConnection($this->target, $this->key);
// Verify that there is a new connection.
$this->assertConnection($id);
// Execute a query.
Database::getConnection($this->target, $this->key)->query('SHOW TABLES')->fetchCol();
// Close the connection.
Database::closeConnection($this->target, $this->key);
// Wait 20ms to give the database engine sufficient time to react.
usleep(20000);
// Verify that we are back to the original connection count.
$this->assertNoConnection($id);
}
/**
* Tests Database::closeConnection() with a select query.
*/
function testOpenSelectQueryClose() {
if ($this->skipTest) {
return;
}
// Add and open a new connection.
$this->addConnection();
$id = $this->getConnectionID();
Database::getConnection($this->target, $this->key);
// Verify that there is a new connection.
$this->assertConnection($id);
// Create a table.
$name = 'foo';
Database::getConnection($this->target, $this->key)->schema()->createTable($name, array(
'fields' => array(
'name' => array(
'type' => 'varchar',
'length' => 255,
),
),
));
// Execute a query.
Database::getConnection($this->target, $this->key)->select('foo', 'f')
->fields('f', array('name'))
->execute()
->fetchAll();
// Drop the table.
Database::getConnection($this->target, $this->key)->schema()->dropTable($name);
// Close the connection.
Database::closeConnection($this->target, $this->key);
// Wait 20ms to give the database engine sufficient time to react.
usleep(20000);
// Verify that we are back to the original connection count.
$this->assertNoConnection($id);
}
}