Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

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
  • Loading branch information...
commit f1270315e9ae8d62f710d34c43e58ad6d8262045 1 parent afa2dbc
Evan Priestley epriestley authored
5 src/infrastructure/storage/connection/AphrontDatabaseConnection.php
View
@@ -28,19 +28,20 @@
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);
4 src/infrastructure/storage/connection/AphrontIsolatedDatabaseConnection.php
View
@@ -38,6 +38,10 @@ public function __construct(array $configuration) {
}
}
+ public function close() {
+ return;
+ }
+
public function escapeString($string) {
return '<S>';
}
4 src/infrastructure/storage/connection/mysql/AphrontMySQLDatabaseConnection.php
View
@@ -34,6 +34,10 @@ public function getAffectedRows() {
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
19 src/infrastructure/storage/connection/mysql/AphrontMySQLDatabaseConnectionBase.php
View
@@ -32,15 +32,24 @@
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 @@ protected function getConfiguration($key, $default = null) {
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 @@ public function executeRawQuery($raw_query) {
// 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;
4 src/infrastructure/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php
View
@@ -36,6 +36,10 @@ public function getAffectedRows() {
return $this->requireConnection()->affected_rows;
}
+ protected function closeConnection() {
+ $this->requireConnection()->close();
+ }
+
protected function connect() {
if (!class_exists('mysqli', false)) {
throw new Exception(
7 src/infrastructure/util/PhabricatorGlobalLock.php
View
@@ -110,11 +110,8 @@ protected function doUnlock() {
'SELECT RELEASE_LOCK(%s)',
'phabricator:'.$this->lockname);
- // TODO: There's no explicit close() method on connections right now. Once
- // we have one, we could close the connection here. Since we don't have
- // 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.
+ $this->conn->close();
+ $this->conn = null;
}
}
Please sign in to comment.
Something went wrong with that request. Please try again.