Allow connections to be closed; close connections for global locks

Summary: Add an explicit close() method to connections and call it in GlobalLock.

Test Plan:
Wrote a script like this:

  $lock = PhabricatorGlobalLock::newLock('test');
  echo "LOCK";
  $lock->lock();
  sleep(10);
  echo "UNLOCK";
  $lock->unlock();
  sleep(9999);

Using `SHOW FULL PROCESSLIST`, verified the connection closed after 10 seconds with both the "MySQL" and "MySQLi" implementations.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1470

Differential Revision: https://secure.phabricator.com/D3035

src/infrastructure/storage/connection/AphrontDatabaseConnection.php

@@ -28,19 +28,20 @@ abstract class AphrontDatabaseConnection {
   abstract public function getAffectedRows();
   abstract public function selectAllResults();
   abstract public function executeRawQuery($raw_query);
+  abstract public function close();
 
   abstract public function escapeString($string);
   abstract public function escapeColumnName($string);
   abstract public function escapeMultilineComment($string);
   abstract public function escapeStringForLikeClause($string);
 
-  public function queryData($pattern/*, $arg, $arg, ... */) {
+  public function queryData($pattern/* , $arg, $arg, ... */) {
     $args = func_get_args();
     array_unshift($args, $this);
     return call_user_func_array('queryfx_all', $args);
   }
 
-  public function query($pattern/*, $arg, $arg, ... */) {
+  public function query($pattern/* , $arg, $arg, ... */) {
     $args = func_get_args();
     array_unshift($args, $this);
     return call_user_func_array('queryfx', $args);

src/infrastructure/storage/connection/AphrontIsolatedDatabaseConnection.php

@@ -38,6 +38,10 @@ final class AphrontIsolatedDatabaseConnection
     }
   }
 
+  public function close() {
+    return;
+  }
+
   public function escapeString($string) {
     return '<S>';
   }

src/infrastructure/storage/connection/mysql/AphrontMySQLDatabaseConnection.php

@@ -34,6 +34,10 @@ final class AphrontMySQLDatabaseConnection
     return mysql_affected_rows($this->requireConnection());
   }
 
+  protected function closeConnection() {
+    mysql_close($this->requireConnection());
+  }
+
   protected function connect() {
     if (!function_exists('mysql_connect')) {
       // We have to '@' the actual call since it can spew all sorts of silly

src/infrastructure/storage/connection/mysql/AphrontMySQLDatabaseConnectionBase.php

@@ -32,15 +32,24 @@ abstract class AphrontMySQLDatabaseConnectionBase
   abstract protected function fetchAssoc($result);
   abstract protected function getErrorCode($connection);
   abstract protected function getErrorDescription($connection);
+  abstract protected function closeConnection();
 
   public function __construct(array $configuration) {
     $this->configuration  = $configuration;
   }
 
+  public function close() {
+    if ($this->connection) {
+      $this->closeConnection();
+      $this->connection = null;
+    }
+  }
+
   public function escapeColumnName($name) {
     return '`'.str_replace('`', '``', $name).'`';
   }
 
+
   public function escapeMultilineComment($comment) {
     // These can either terminate a comment, confuse the hell out of the parser,
     // make MySQL execute the comment as a query, or, in the case of semicolon,
@@ -78,12 +87,6 @@ abstract class AphrontMySQLDatabaseConnectionBase
     return idx($this->configuration, $key, $default);
   }
 
-  private function closeConnection() {
-    if ($this->connection) {
-      $this->connection = null;
-    }
-  }
-
   private function establishConnection() {
     $start = microtime(true);
 
@@ -189,12 +192,12 @@ abstract class AphrontMySQLDatabaseConnectionBase
           // We can't close the connection before this because
           // isInsideTransaction() and getTransactionState() depend on the
           // connection.
-          $this->closeConnection();
+          $this->close();
 
           throw $ex;
         }
 
-        $this->closeConnection();
+        $this->close();
 
         if (!$retries) {
           throw $ex;

src/infrastructure/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php

@@ -36,6 +36,10 @@ final class AphrontMySQLiDatabaseConnection
     return $this->requireConnection()->affected_rows;
   }
 
+  protected function closeConnection() {
+    $this->requireConnection()->close();
+  }
+
   protected function connect() {
     if (!class_exists('mysqli', false)) {
       throw new Exception(

src/infrastructure/util/PhabricatorGlobalLock.php

@@ -110,11 +110,8 @@ final class PhabricatorGlobalLock extends PhutilLock {
       'SELECT RELEASE_LOCK(%s)',
       'phabricator:'.$this->lockname);
 
-    // TODO: There's no explicit close() method on connections right now. Once
+    $this->conn->close();
-    // we have one, we could close the connection here. Since we don't have
+    $this->conn = null;
-    // such a method, we need to keep the connection around in case lock() is
-    // called again, so that long-running daemons don't gradually open
-    // an unbounded number of connections.
   }
 
 }