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 dc1570e..9315daf 100644 (file)
--- 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 3a8f737..29106ab 100644 (file)
--- 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 4d9891e..e84f264 100644 (file)
--- 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;
        }
 
        /**