From: Aaron Schulz <aschulz@wikimedia.org>
Date: Sat, 30 Jan 2016 17:17:17 +0000 (-0800)
Subject: Do not auto-reconnect to DBs if named locks where lost
X-Git-Tag: 1.31.0-rc.0~8136^2
X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/password.php?a=commitdiff_plain;h=79d12a284b8baf05d6cb600e616e576f1977e6a9;p=lhc%2Fweb%2Fwiklou.git

Do not auto-reconnect to DBs if named locks where lost

Otherwise, transactions might be committed without cooperative
locks that were supposed to be present. This is similar to the
logic of not auto-reconnecting if a transaction was started to
avoid committing incomplete changes.

Change-Id: Ia7bc6b188bb5ee53a5bf7c5a30718bc7c4dd0ba9
---

diff --git a/includes/db/Database.php b/includes/db/Database.php
index dc1570e1a0..9315daf85e 100644
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -155,6 +155,9 @@ abstract class DatabaseBase implements IDatabase {
 	 */
 	private $mTrxWriteDuration = 0.0;
 
+	/** @var array Map of (name => 1) for locks obtained via lock() */
+	private $mNamedLocksHeld = array();
+
 	/** @var IDatabase|null Lazy handle to the master DB this server replicates from */
 	private $lazyMasterHandle;
 
@@ -871,7 +874,7 @@ abstract class DatabaseBase implements IDatabase {
 				$msg = __METHOD__ . ": lost connection to $server; reconnected";
 				wfDebugLog( 'DBPerformance', "$msg:\n" . wfBacktrace( true ) );
 
-				if ( $hadTrx ) {
+				if ( $hadTrx || $this->mNamedLocksHeld ) {
 					# Leave $ret as false and let an error be reported.
 					# Callers may catch the exception and continue to use the DB.
 					$this->reportQueryError( $lastError, $lastErrno, $sql, $fname, $tempIgnore );
@@ -3160,10 +3163,14 @@ abstract class DatabaseBase implements IDatabase {
 	}
 
 	public function lock( $lockName, $method, $timeout = 5 ) {
+		$this->mNamedLocksHeld[$lockName] = 1;
+
 		return true;
 	}
 
 	public function unlock( $lockName, $method ) {
+		unset( $this->mNamedLocksHeld[$lockName] );
+
 		return true;
 	}
 
diff --git a/includes/db/DatabaseMysqlBase.php b/includes/db/DatabaseMysqlBase.php
index 3a8f7373b6..29106abedd 100644
--- a/includes/db/DatabaseMysqlBase.php
+++ b/includes/db/DatabaseMysqlBase.php
@@ -932,12 +932,13 @@ abstract class DatabaseMysqlBase extends Database {
 		$row = $this->fetchObject( $result );
 
 		if ( $row->lockstatus == 1 ) {
+			parent::lock( $lockName, $method, $timeout ); // record
 			return true;
-		} else {
-			wfDebug( __METHOD__ . " failed to acquire lock\n" );
-
-			return false;
 		}
+
+		wfDebug( __METHOD__ . " failed to acquire lock\n" );
+
+		return false;
 	}
 
 	/**
@@ -952,7 +953,14 @@ abstract class DatabaseMysqlBase extends Database {
 		$result = $this->query( "SELECT RELEASE_LOCK($lockName) as lockstatus", $method );
 		$row = $this->fetchObject( $result );
 
-		return ( $row->lockstatus == 1 );
+		if ( $row->lockstatus == 1 ) {
+			parent::unlock( $lockName, $method ); // record
+			return true;
+		}
+
+		wfDebug( __METHOD__ . " failed to release lock\n" );
+
+		return false;
 	}
 
 	private function makeLockName( $lockName ) {
diff --git a/includes/db/DatabasePostgres.php b/includes/db/DatabasePostgres.php
index 4d9891e886..e84f264d1e 100644
--- a/includes/db/DatabasePostgres.php
+++ b/includes/db/DatabasePostgres.php
@@ -1581,11 +1581,13 @@ SQL;
 				"SELECT pg_try_advisory_lock($key) AS lockstatus", $method );
 			$row = $this->fetchObject( $result );
 			if ( $row->lockstatus === 't' ) {
+				parent::lock( $lockName, $method, $timeout ); // record
 				return true;
 			} else {
 				sleep( 1 );
 			}
 		}
+
 		wfDebug( __METHOD__ . " failed to acquire lock\n" );
 
 		return false;
@@ -1603,7 +1605,14 @@ SQL;
 		$result = $this->query( "SELECT pg_advisory_unlock($key) as lockstatus", $method );
 		$row = $this->fetchObject( $result );
 
-		return ( $row->lockstatus === 't' );
+		if ( $row->lockstatus === 't' ) {
+			parent::unlock( $lockName, $method ); // record
+			return true;
+		}
+
+		wfDebug( __METHOD__ . " failed to release lock\n" );
+
+		return false;
 	}
 
 	/**