I understand that it should close the connection, since it unsets the connection object, but my crude but straightforward test suggest that it doesn't.

In your local set-up, set the MAX_CONNECTIONS to a very low number (5 is good). Then create a good 10 empty DBs in your MySQL instance. To keep them easy to remember, manipulate and then delete, call them a1, a2, a3, etc.

Now, if you circumvent the drupal API. And just open a bunch of connections to these DB's using just the PDO. It should look like this:

for($i=1; $i<=10; $i++) {
  $dbh[$i] = new PDO('mysql:host=localhost;dbname=a' . $i, YOUR_DB_USER_HERE, YOUR_DB_PASS_HERE);
  // use the connection here
  $dbh[$i]->exec("SHOW tables");
}

Throw that into a devel window (replace DB creds with your own (privileged) DB user) and your site should crash from too many connections. That makes sense.
But add one more line to this, one that unsets the connection object, and you are safe. No PDO exception for too many connections.

for($i=1; $i<=10; $i++) {
  $dbh[$i] = new PDO('mysql:host=localhost;dbname=a' . $i, YOUR_DB_USER_HERE, YOUR_DB_PASS_HERE);
  // use the connection here
  $dbh[$i]->exec("SHOW tables");
  $dbh[$i] = NULL; // close it
}

Which all makes sense. From PHP.net (http://php.net/manual/en/pdo.connections.php)

To close the connection, you need to destroy the object by ensuring that all remaining references to it are deleted--you do this by assigning NULL to the variable that holds the object.

But if we try this in Drupal, however, the connections simply don't close.

First, add the new dummy DBs to drupal's global $database variable. (I like to just throw the following into settings.php)

for($i=1; $i<=10; $i++){
$databases['a' . $i]['default'] = 
    array (
      'database' => 'a' . $i,
      'username' => 'root',
      'password' => 'root',
      'host' => 'localhost',
      'port' => '',
      'driver' => 'mysql',
      'prefix' => '',
    );
}

Then insert the following into a php window.

for($i=1; $i<=10; $i++) {
  db_set_active('a' . $i);
  db_query("SHOW TABLES");
  db_set_active();
}

Your site will again crash, which makes sense. But if we try to explicitly close the connections as we did above with just the PDO ....

for($i=1; $i<=10; $i++) {
  db_set_active('a' . $i);
  db_query("SHOW TABLES");
  db_set_active();
  Database::closeConnection(NULL, 'a' . $i);
} 

You will still reach too many connections. I'm certainly no expert in PHP5 object management, but it seems that the key line of closeConnection

unset(self::$connections[$key]);

does not completely delete every reference to the connection object. I don't know if it's because another method (openConnection) actually creates the object, or if it's because we're using the Database abstract class to do all this.

Comments

tim.plunkett’s picture

Version: 7.15 » 8.x-dev
Priority: Major » Normal
Issue tags: +Needs backport to D7

Moving to D8, this doesn't seem like a major bug

Anonymous’s picture

From the supplied link in the node we also see:

Many web applications will benefit from making persistent connections to database servers. Persistent connections are not closed at the end of the script, but are cached and re-used when another script requests a connection using the same credentials. The persistent connection cache allows you to avoid the overhead of establishing a new connection every time a script needs to talk to a database, resulting in a faster web application.

As well persistent connections is a configurable item - see http://dev.mysql.com/doc/refman/5.1/en/apis-php-mysql.setup.html for more.

sun’s picture

Title: Database::closeConnection does not actually close connection » Database::closeConnection() does not actually close connection
Priority: Normal » Critical
Issue tags: +Testing system

Bumping to critical, since this bug is causing random test failures on HEAD currently.

TestBase::tearDown() calls Database::removeConnection(), which is supposed to also close the connection.

Manually injecting an additional Database::closeConnection() does not change the situation.

Monitoring the result of SHOW PROCESSLIST during a test run shows that new connections are stacking up for every test method that is executed in a test case.

> php core\scripts\run-tests.sh --php php --url http://drupal8.test/ --class Drupal\system\Tests\Common\JavaScriptTest

Drupal test run
---------------

Tests to be run:
 - JavaScript (Drupal\system\Tests\Common\JavaScriptTest)

Test run started:
 Thursday, September 27, 2012 - 00:31

Test summary
------------

exception 'PDOException' with message 'SQLSTATE[08004] [1040] Too many connections' in \core\lib\Drupal\Core\Database\Connection.php:146
Stack trace:
#0 \core\lib\Drupal\Core\Database\Connection.php(146): PDO->__construct('mysql:host=loca...', 'username', 'password', Array)
#1 \core\lib\Drupal\Core\Database\Driver\mysql\Connection.php(62): Drupal\Core\Database\Connection->__construct('mysql:host=loca...', 'username', 'password', Array)
#2 \core\lib\Drupal\Core\Database\Database.php(378): Drupal\Core\Database\Driver\mysql\Connection->__construct(Array)
#3 \core\lib\Drupal\Core\Database\Database.php(171): Drupal\Core\Database\Database::openConnection('default', 'default')
#4 \core\includes\database.inc(407): Drupal\Core\Database\Database::getConnection('default')
#5 \core\includes\menu.inc(2663): db_transaction()
#6 \core\includes\menu.inc(455): menu_router_rebuild()
#7 \core\includes\form.inc(740): menu_get_item()
#8 \core\includes\form.inc(702): drupal_retrieve_form('install_setting...', Array)
#9 \core\includes\install.core.inc(504): drupal_form_submit('install_setting...', Array)
#10 \core\includes\install.core.inc(432): install_run_task(Array, Array)
#11 \core\includes\install.core.inc(85): install_run_tasks(Array)
#12 \core\modules\simpletest\lib\Drupal\simpletest\WebTestBase.php(687): install_drupal(Array)
#13 \core\modules\system\lib\Drupal\system\Tests\Common\JavaScriptTest.php(38): Drupal\simpletest\WebTestBase->setUp()
#14 \core\modules\simpletest\lib\Drupal\simpletest\TestBase.php(584): Drupal\system\Tests\Common\JavaScriptTest->setUp()
#15 \core\scripts\run-tests.sh(381): Drupal\simpletest\TestBase->run()
#16 \core\scripts\run-tests.sh(22): simpletest_script_run_one_test('150', 'Drupal\system\T...')
#17 {main}
FATAL Drupal\system\Tests\Common\JavaScriptTest: test runner returned a non-zero error code (1).
- Found database prefix 'simpletest643317' for test ID 150.
- Removed test files directory.

Test run duration: 1 min 39 sec
sun’s picture

crookednumber’s picture

Thanks, Sun, for your input -- and the dupe link. Most helpful.

crookednumber’s picture

For anyone coming to this thread with a similar problem, read the ticket that this one dupes #843114: DatabaseConnection::__construct() and DatabaseConnection_mysql::__construct() leaks $this (Too many connections)

TL;DR: You can get closeConnection() to actually work if you follow the hack outlined by quicksketch in this comment: http://drupal.org/node/843114#comment-6075722

Applied to the code sample above, it would look something like:

for($i=1; $i<=10; $i++) {
  db_set_active('a' . $i);
  db_query("SHOW TABLES");
  db_set_active();
  $connection = Database::getConnection('default', 'a' . $i);
  $connection->setAttribute(PDO::ATTR_STATEMENT_CLASS, array('DatabaseStatementBase', array()));
  Database::closeConnection(NULL, 'a' . $i);
} 

Yeah, it'll iterate over the loop a little more slowly. But it shouldn't crash from too many connections.