Update to MediaWiki 1.24.1

9 files changed, 192 insertions, 405 deletions

diff --git a/includes/filebackend/lockmanager/DBLockManager.php b/includes/filebackend/lockmanager/DBLockManager.php
index 3e934ba5..450ccc82 100644
--- a/includes/filebackend/lockmanager/DBLockManager.php
+++ b/includes/filebackend/lockmanager/DBLockManager.php
@@ -37,7 +37,7 @@
  * @since 1.19
  */
 abstract class DBLockManager extends QuorumLockManager {
-	/** @var Array Map of DB names to server config */
+	/** @var array Map of DB names to server config */
 	protected $dbServers; // (DB name => server config array)
 	/** @var BagOStuff */
 	protected $statusCache;
@@ -46,13 +46,13 @@ abstract class DBLockManager extends QuorumLockManager {
 	protected $safeDelay; // integer number of seconds
 
 	protected $session = 0; // random integer
-	/** @var Array Map Database connections (DB name => Database) */
+	/** @var array Map Database connections (DB name => Database) */
 	protected $conns = array();
 
 	/**
 	 * Construct a new instance from configuration.
 	 *
-	 * $config paramaters include:
+	 * @param array $config Paramaters include:
 	 *   - dbServers   : Associative array of DB names to server configuration.
 	 *                   Configuration is an associative array that includes:
 	 *                     - host        : DB server name
@@ -70,8 +70,6 @@ abstract class DBLockManager extends QuorumLockManager {
 	 *   - lockExpiry  : Lock timeout (seconds) for dropped connections. [optional]
 	 *                   This tells the DB server how long to wait before assuming
 	 *                   connection failure and releasing all the locks for a session.
-	 *
-	 * @param array $config
 	 */
 	public function __construct( array $config ) {
 		parent::__construct( $config );
@@ -110,12 +108,13 @@ abstract class DBLockManager extends QuorumLockManager {
 		$this->session = wfRandomString( 31 );
 	}
 
-	// @TODO: change this code to work in one batch
+	// @todo change this code to work in one batch
 	protected function getLocksOnServer( $lockSrv, array $pathsByType ) {
 		$status = Status::newGood();
 		foreach ( $pathsByType as $type => $paths ) {
 			$status->merge( $this->doGetLocksOnServer( $lockSrv, $paths, $type ) );
 		}
+
 		return $status;
 	}
 
@@ -125,6 +124,7 @@ abstract class DBLockManager extends QuorumLockManager {
 
 	/**
 	 * @see QuorumLockManager::isServerUp()
+	 * @param string $lockSrv
 	 * @return bool
 	 */
 	protected function isServerUp( $lockSrv ) {
@@ -135,15 +135,17 @@ abstract class DBLockManager extends QuorumLockManager {
 			$this->getConnection( $lockSrv );
 		} catch ( DBError $e ) {
 			$this->cacheRecordFailure( $lockSrv );
+
 			return false; // failed to connect
 		}
+
 		return true;
 	}
 
 	/**
 	 * Get (or reuse) a connection to a lock DB
 	 *
-	 * @param $lockDb string
+	 * @param string $lockDb
 	 * @return DatabaseBase
 	 * @throws DBError
 	 */
@@ -175,24 +177,25 @@ abstract class DBLockManager extends QuorumLockManager {
 		if ( !$this->conns[$lockDb]->trxLevel() ) {
 			$this->conns[$lockDb]->begin( __METHOD__ ); // start transaction
 		}
+
 		return $this->conns[$lockDb];
 	}
 
 	/**
 	 * Do additional initialization for new lock DB connection
 	 *
-	 * @param $lockDb string
-	 * @param $db DatabaseBase
-	 * @return void
+	 * @param string $lockDb
+	 * @param DatabaseBase $db
 	 * @throws DBError
 	 */
-	protected function initConnection( $lockDb, DatabaseBase $db ) {}
+	protected function initConnection( $lockDb, DatabaseBase $db ) {
+	}
 
 	/**
 	 * Checks if the DB has not recently had connection/query errors.
 	 * This just avoids wasting time on doomed connection attempts.
 	 *
-	 * @param $lockDb string
+	 * @param string $lockDb
 	 * @return bool
 	 */
 	protected function cacheCheckFailures( $lockDb ) {
@@ -204,7 +207,7 @@ abstract class DBLockManager extends QuorumLockManager {
 	/**
 	 * Log a lock request failure to the cache
 	 *
-	 * @param $lockDb string
+	 * @param string $lockDb
 	 * @return bool Success
 	 */
 	protected function cacheRecordFailure( $lockDb ) {
@@ -216,7 +219,7 @@ abstract class DBLockManager extends QuorumLockManager {
 	/**
 	 * Get a cache key for recent query misses for a DB
 	 *
-	 * @param $lockDb string
+	 * @param string $lockDb
 	 * @return string
 	 */
 	protected function getMissKey( $lockDb ) {
@@ -242,7 +245,7 @@ abstract class DBLockManager extends QuorumLockManager {
  * @ingroup LockManager
  */
 class MySqlLockManager extends DBLockManager {
-	/** @var Array Mapping of lock types to the type actually used */
+	/** @var array Mapping of lock types to the type actually used */
 	protected $lockTypeMap = array(
 		self::LOCK_SH => self::LOCK_SH,
 		self::LOCK_UW => self::LOCK_SH,
@@ -250,8 +253,8 @@ class MySqlLockManager extends DBLockManager {
 	);
 
 	/**
-	 * @param $lockDb string
-	 * @param $db DatabaseBase
+	 * @param string $lockDb
+	 * @param DatabaseBase $db
 	 */
 	protected function initConnection( $lockDb, DatabaseBase $db ) {
 		# Let this transaction see lock rows from other transactions
@@ -263,6 +266,9 @@ class MySqlLockManager extends DBLockManager {
 	 * This does not use GET_LOCK() per http://bugs.mysql.com/bug.php?id=1118.
 	 *
 	 * @see DBLockManager::getLocksOnServer()
+	 * @param string $lockSrv
+	 * @param array $paths
+	 * @param string $type
 	 * @return Status
 	 */
 	protected function doGetLocksOnServer( $lockSrv, array $paths, $type ) {
@@ -359,7 +365,7 @@ class MySqlLockManager extends DBLockManager {
  * @ingroup LockManager
  */
 class PostgreSqlLockManager extends DBLockManager {
-	/** @var Array Mapping of lock types to the type actually used */
+	/** @var array Mapping of lock types to the type actually used */
 	protected $lockTypeMap = array(
 		self::LOCK_SH => self::LOCK_SH,
 		self::LOCK_UW => self::LOCK_SH,
@@ -374,7 +380,7 @@ class PostgreSqlLockManager extends DBLockManager {
 
 		$db = $this->getConnection( $lockSrv ); // checked in isServerUp()
 		$bigints = array_unique( array_map(
-			function( $key ) {
+			function ( $key ) {
 				return wfBaseConvert( substr( $key, 0, 15 ), 16, 10 );
 			},
 			array_map( array( $this, 'sha1Base16Absolute' ), $paths )
diff --git a/includes/filebackend/lockmanager/FSLockManager.php b/includes/filebackend/lockmanager/FSLockManager.php
index eacba704..bce6b34c 100644
--- a/includes/filebackend/lockmanager/FSLockManager.php
+++ b/includes/filebackend/lockmanager/FSLockManager.php
@@ -34,7 +34,7 @@
  * @since 1.19
  */
 class FSLockManager extends LockManager {
-	/** @var Array Mapping of lock types to the type actually used */
+	/** @var array Mapping of lock types to the type actually used */
 	protected $lockTypeMap = array(
 		self::LOCK_SH => self::LOCK_SH,
 		self::LOCK_UW => self::LOCK_SH,
@@ -43,16 +43,14 @@ class FSLockManager extends LockManager {
 
 	protected $lockDir; // global dir for all servers
 
-	/** @var Array Map of (locked key => lock file handle) */
+	/** @var array Map of (locked key => lock file handle) */
 	protected $handles = array();
 
 	/**
 	 * Construct a new instance from configuration.
 	 *
-	 * $config includes:
+	 * @param array $config Includes:
 	 *   - lockDirectory : Directory containing the lock files
-	 *
-	 * @param array $config
 	 */
 	function __construct( array $config ) {
 		parent::__construct( $config );
@@ -62,8 +60,8 @@ class FSLockManager extends LockManager {
 
 	/**
 	 * @see LockManager::doLock()
-	 * @param $paths array
-	 * @param $type int
+	 * @param array $paths
+	 * @param int $type
 	 * @return Status
 	 */
 	protected function doLock( array $paths, $type ) {
@@ -77,6 +75,7 @@ class FSLockManager extends LockManager {
 			} else {
 				// Abort and unlock everything
 				$status->merge( $this->doUnlock( $lockedPaths, $type ) );
+
 				return $status;
 			}
 		}
@@ -86,8 +85,8 @@ class FSLockManager extends LockManager {
 
 	/**
 	 * @see LockManager::doUnlock()
-	 * @param $paths array
-	 * @param $type int
+	 * @param array $paths
+	 * @param int $type
 	 * @return Status
 	 */
 	protected function doUnlock( array $paths, $type ) {
@@ -103,8 +102,8 @@ class FSLockManager extends LockManager {
 	/**
 	 * Lock a single resource key
 	 *
-	 * @param $path string
-	 * @param $type integer
+	 * @param string $path
+	 * @param int $type
 	 * @return Status
 	 */
 	protected function doSingleLock( $path, $type ) {
@@ -148,8 +147,8 @@ class FSLockManager extends LockManager {
 	/**
 	 * Unlock a single resource key
 	 *
-	 * @param $path string
-	 * @param $type integer
+	 * @param string $path
+	 * @param int $type
 	 * @return Status
 	 */
 	protected function doSingleUnlock( $path, $type ) {
@@ -191,8 +190,8 @@ class FSLockManager extends LockManager {
 	}
 
 	/**
-	 * @param $path string
-	 * @param $handlesToClose array
+	 * @param string $path
+	 * @param array $handlesToClose
 	 * @return Status
 	 */
 	private function closeLockHandles( $path, array $handlesToClose ) {
@@ -205,11 +204,12 @@ class FSLockManager extends LockManager {
 				$status->warning( 'lockmanager-fail-closelock', $path );
 			}
 		}
+
 		return $status;
 	}
 
 	/**
-	 * @param $path string
+	 * @param string $path
 	 * @return Status
 	 */
 	private function pruneKeyLockFiles( $path ) {
@@ -221,12 +221,13 @@ class FSLockManager extends LockManager {
 			}
 			unset( $this->handles[$path] );
 		}
+
 		return $status;
 	}
 
 	/**
 	 * Get the path to the lock file for a key
-	 * @param $path string
+	 * @param string $path
 	 * @return string
 	 */
 	protected function getLockPath( $path ) {
diff --git a/includes/filebackend/lockmanager/LSLockManager.php b/includes/filebackend/lockmanager/LSLockManager.php
deleted file mode 100644
index 97de8dca..00000000
--- a/includes/filebackend/lockmanager/LSLockManager.php
+++ /dev/null
@@ -1,218 +0,0 @@
-<?php
-/**
- * Version of LockManager based on using lock daemon servers.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
- * http://www.gnu.org/copyleft/gpl.html
- *
- * @file
- * @ingroup LockManager
- */
-
-/**
- * Manage locks using a lock daemon server.
- *
- * Version of LockManager based on using lock daemon servers.
- * This is meant for multi-wiki systems that may share files.
- * All locks are non-blocking, which avoids deadlocks.
- *
- * All lock requests for a resource, identified by a hash string, will map
- * to one bucket. Each bucket maps to one or several peer servers, each
- * running LockServerDaemon.php, listening on a designated TCP port.
- * A majority of peers must agree for a lock to be acquired.
- *
- * @ingroup LockManager
- * @since 1.19
- */
-class LSLockManager extends QuorumLockManager {
-	/** @var Array Mapping of lock types to the type actually used */
-	protected $lockTypeMap = array(
-		self::LOCK_SH => self::LOCK_SH,
-		self::LOCK_UW => self::LOCK_SH,
-		self::LOCK_EX => self::LOCK_EX
-	);
-
-	/** @var Array Map of server names to server config */
-	protected $lockServers; // (server name => server config array)
-
-	/** @var Array Map Server connections (server name => resource) */
-	protected $conns = array();
-
-	protected $connTimeout; // float number of seconds
-	protected $session = ''; // random SHA-1 string
-
-	/**
-	 * Construct a new instance from configuration.
-	 *
-	 * $config paramaters include:
-	 *   - lockServers  : Associative array of server names to configuration.
-	 *                    Configuration is an associative array that includes:
-	 *                      - host    : IP address/hostname
-	 *                      - port    : TCP port
-	 *                      - authKey : Secret string the lock server uses
-	 *   - srvsByBucket : Array of 1-16 consecutive integer keys, starting from 0,
-	 *                    each having an odd-numbered list of server names (peers) as values.
-	 *   - connTimeout  : Lock server connection attempt timeout. [optional]
-	 *
-	 * @param array $config
-	 */
-	public function __construct( array $config ) {
-		parent::__construct( $config );
-
-		$this->lockServers = $config['lockServers'];
-		// Sanitize srvsByBucket config to prevent PHP errors
-		$this->srvsByBucket = array_filter( $config['srvsByBucket'], 'is_array' );
-		$this->srvsByBucket = array_values( $this->srvsByBucket ); // consecutive
-
-		if ( isset( $config['connTimeout'] ) ) {
-			$this->connTimeout = $config['connTimeout'];
-		} else {
-			$this->connTimeout = 3; // use some sane amount
-		}
-
-		$this->session = wfRandomString( 32 ); // 128 bits
-	}
-
-	/**
-	 * @see QuorumLockManager::getLocksOnServer()
-	 * @return Status
-	 */
-	protected function getLocksOnServer( $lockSrv, array $paths, $type ) {
-		$status = Status::newGood();
-
-		// Send out the command and get the response...
-		$type = ( $type == self::LOCK_SH ) ? 'SH' : 'EX';
-		$keys = array_unique( array_map( array( $this, 'sha1Base36Absolute' ), $paths ) );
-		$response = $this->sendCommand( $lockSrv, 'ACQUIRE', $type, $keys );
-
-		if ( $response !== 'ACQUIRED' ) {
-			foreach ( $paths as $path ) {
-				$status->fatal( 'lockmanager-fail-acquirelock', $path );
-			}
-		}
-
-		return $status;
-	}
-
-	/**
-	 * @see QuorumLockManager::freeLocksOnServer()
-	 * @return Status
-	 */
-	protected function freeLocksOnServer( $lockSrv, array $paths, $type ) {
-		$status = Status::newGood();
-
-		// Send out the command and get the response...
-		$type = ( $type == self::LOCK_SH ) ? 'SH' : 'EX';
-		$keys = array_unique( array_map( array( $this, 'sha1Base36Absolute' ), $paths ) );
-		$response = $this->sendCommand( $lockSrv, 'RELEASE', $type, $keys );
-
-		if ( $response !== 'RELEASED' ) {
-			foreach ( $paths as $path ) {
-				$status->fatal( 'lockmanager-fail-releaselock', $path );
-			}
-		}
-
-		return $status;
-	}
-
-	/**
-	 * @see QuorumLockManager::releaseAllLocks()
-	 * @return Status
-	 */
-	protected function releaseAllLocks() {
-		$status = Status::newGood();
-
-		foreach ( $this->conns as $lockSrv => $conn ) {
-			$response = $this->sendCommand( $lockSrv, 'RELEASE_ALL', '', array() );
-			if ( $response !== 'RELEASED_ALL' ) {
-				$status->fatal( 'lockmanager-fail-svr-release', $lockSrv );
-			}
-		}
-
-		return $status;
-	}
-
-	/**
-	 * @see QuorumLockManager::isServerUp()
-	 * @return bool
-	 */
-	protected function isServerUp( $lockSrv ) {
-		return (bool)$this->getConnection( $lockSrv );
-	}
-
-	/**
-	 * Send a command and get back the response
-	 *
-	 * @param $lockSrv string
-	 * @param $action string
-	 * @param $type string
-	 * @param $values Array
-	 * @return string|bool
-	 */
-	protected function sendCommand( $lockSrv, $action, $type, $values ) {
-		$conn = $this->getConnection( $lockSrv );
-		if ( !$conn ) {
-			return false; // no connection
-		}
-		$authKey = $this->lockServers[$lockSrv]['authKey'];
-		// Build of the command as a flat string...
-		$values = implode( '|', $values );
-		$key = hash_hmac( 'sha1', "{$this->session}\n{$action}\n{$type}\n{$values}", $authKey );
-		// Send out the command...
-		if ( fwrite( $conn, "{$this->session}:$key:$action:$type:$values\n" ) === false ) {
-			return false;
-		}
-		// Get the response...
-		$response = fgets( $conn );
-		if ( $response === false ) {
-			return false;
-		}
-		return trim( $response );
-	}
-
-	/**
-	 * Get (or reuse) a connection to a lock server
-	 *
-	 * @param $lockSrv string
-	 * @return resource
-	 */
-	protected function getConnection( $lockSrv ) {
-		if ( !isset( $this->conns[$lockSrv] ) ) {
-			$cfg = $this->lockServers[$lockSrv];
-			wfSuppressWarnings();
-			$errno = $errstr = '';
-			$conn = fsockopen( $cfg['host'], $cfg['port'], $errno, $errstr, $this->connTimeout );
-			wfRestoreWarnings();
-			if ( $conn === false ) {
-				return null;
-			}
-			$sec = floor( $this->connTimeout );
-			$usec = floor( ( $this->connTimeout - floor( $this->connTimeout ) ) * 1e6 );
-			stream_set_timeout( $conn, $sec, $usec );
-			$this->conns[$lockSrv] = $conn;
-		}
-		return $this->conns[$lockSrv];
-	}
-
-	/**
-	 * Make sure remaining locks get cleared for sanity
-	 */
-	function __destruct() {
-		$this->releaseAllLocks();
-		foreach ( $this->conns as $conn ) {
-			fclose( $conn );
-		}
-	}
-}
diff --git a/includes/filebackend/lockmanager/LockManager.php b/includes/filebackend/lockmanager/LockManager.php
index dad8a624..df8d2d4f 100644
--- a/includes/filebackend/lockmanager/LockManager.php
+++ b/includes/filebackend/lockmanager/LockManager.php
@@ -43,14 +43,14 @@
  * @since 1.19
  */
 abstract class LockManager {
-	/** @var Array Mapping of lock types to the type actually used */
+	/** @var array Mapping of lock types to the type actually used */
 	protected $lockTypeMap = array(
 		self::LOCK_SH => self::LOCK_SH,
 		self::LOCK_UW => self::LOCK_EX, // subclasses may use self::LOCK_SH
 		self::LOCK_EX => self::LOCK_EX
 	);
 
-	/** @var Array Map of (resource path => lock type => count) */
+	/** @var array Map of (resource path => lock type => count) */
 	protected $locksHeld = array();
 
 	protected $domain; // string; domain (usually wiki ID)
@@ -64,12 +64,10 @@ abstract class LockManager {
 	/**
 	 * Construct a new instance from configuration
 	 *
-	 * $config paramaters include:
+	 * @param array $config Paramaters include:
 	 *   - domain  : Domain (usually wiki ID) that all resources are relative to [optional]
 	 *   - lockTTL : Age (in seconds) at which resource locks should expire.
 	 *               This only applies if locks are not tied to a connection/process.
-	 *
-	 * @param $config Array
 	 */
 	public function __construct( array $config ) {
 		$this->domain = isset( $config['domain'] ) ? $config['domain'] : wfWikiID();
@@ -87,8 +85,8 @@ abstract class LockManager {
 	 * Lock the resources at the given abstract paths
 	 *
 	 * @param array $paths List of resource names
-	 * @param $type integer LockManager::LOCK_* constant
-	 * @param integer $timeout Timeout in seconds (0 means non-blocking) (since 1.21)
+	 * @param int $type LockManager::LOCK_* constant
+	 * @param int $timeout Timeout in seconds (0 means non-blocking) (since 1.21)
 	 * @return Status
 	 */
 	final public function lock( array $paths, $type = self::LOCK_EX, $timeout = 0 ) {
@@ -99,7 +97,7 @@ abstract class LockManager {
 	 * Lock the resources at the given abstract paths
 	 *
 	 * @param array $pathsByType Map of LockManager::LOCK_* constants to lists of paths
-	 * @param integer $timeout Timeout in seconds (0 means non-blocking) (since 1.21)
+	 * @param int $timeout Timeout in seconds (0 means non-blocking) (since 1.21)
 	 * @return Status
 	 * @since 1.22
 	 */
@@ -119,6 +117,7 @@ abstract class LockManager {
 			$elapsed = microtime( true ) - $start;
 		} while ( $elapsed < $timeout && $elapsed >= 0 );
 		wfProfileOut( __METHOD__ );
+
 		return $status;
 	}
 
@@ -126,7 +125,7 @@ abstract class LockManager {
 	 * Unlock the resources at the given abstract paths
 	 *
 	 * @param array $paths List of paths
-	 * @param $type integer LockManager::LOCK_* constant
+	 * @param int $type LockManager::LOCK_* constant
 	 * @return Status
 	 */
 	final public function unlock( array $paths, $type = self::LOCK_EX ) {
@@ -145,6 +144,7 @@ abstract class LockManager {
 		$pathsByType = $this->normalizePathsByType( $pathsByType );
 		$status = $this->doUnlockByType( $pathsByType );
 		wfProfileOut( __METHOD__ );
+
 		return $status;
 	}
 
@@ -153,7 +153,7 @@ abstract class LockManager {
 	 * Before hashing, the path will be prefixed with the domain ID.
 	 * This should be used interally for lock key or file names.
 	 *
-	 * @param $path string
+	 * @param string $path
 	 * @return string
 	 */
 	final protected function sha1Base36Absolute( $path ) {
@@ -165,7 +165,7 @@ abstract class LockManager {
 	 * Before hashing, the path will be prefixed with the domain ID.
 	 * This should be used interally for lock key or file names.
 	 *
-	 * @param $path string
+	 * @param string $path
 	 * @return string
 	 */
 	final protected function sha1Base16Absolute( $path ) {
@@ -176,8 +176,8 @@ abstract class LockManager {
 	 * Normalize the $paths array by converting LOCK_UW locks into the
 	 * appropriate type and removing any duplicated paths for each lock type.
 	 *
-	 * @param array $paths Map of LockManager::LOCK_* constants to lists of paths
-	 * @return Array
+	 * @param array $pathsByType Map of LockManager::LOCK_* constants to lists of paths
+	 * @return array
 	 * @since 1.22
 	 */
 	final protected function normalizePathsByType( array $pathsByType ) {
@@ -185,12 +185,13 @@ abstract class LockManager {
 		foreach ( $pathsByType as $type => $paths ) {
 			$res[$this->lockTypeMap[$type]] = array_unique( $paths );
 		}
+
 		return $res;
 	}
 
 	/**
 	 * @see LockManager::lockByType()
-	 * @param array $paths Map of LockManager::LOCK_* constants to lists of paths
+	 * @param array $pathsByType Map of LockManager::LOCK_* constants to lists of paths
 	 * @return Status
 	 * @since 1.22
 	 */
@@ -203,12 +204,13 @@ abstract class LockManager {
 				$lockedByType[$type] = $paths;
 			} else {
 				// Release the subset of locks that were acquired
-				foreach ( $lockedByType as $type => $paths ) {
-					$status->merge( $this->doUnlock( $paths, $type ) );
+				foreach ( $lockedByType as $lType => $lPaths ) {
+					$status->merge( $this->doUnlock( $lPaths, $lType ) );
 				}
 				break;
 			}
 		}
+
 		return $status;
 	}
 
@@ -216,14 +218,14 @@ abstract class LockManager {
 	 * Lock resources with the given keys and lock type
 	 *
 	 * @param array $paths List of paths
-	 * @param $type integer LockManager::LOCK_* constant
+	 * @param int $type LockManager::LOCK_* constant
 	 * @return Status
 	 */
 	abstract protected function doLock( array $paths, $type );
 
 	/**
 	 * @see LockManager::unlockByType()
-	 * @param array $paths Map of LockManager::LOCK_* constants to lists of paths
+	 * @param array $pathsByType Map of LockManager::LOCK_* constants to lists of paths
 	 * @return Status
 	 * @since 1.22
 	 */
@@ -232,6 +234,7 @@ abstract class LockManager {
 		foreach ( $pathsByType as $type => $paths ) {
 			$status->merge( $this->doUnlock( $paths, $type ) );
 		}
+
 		return $status;
 	}
 
@@ -239,7 +242,7 @@ abstract class LockManager {
 	 * Unlock resources with the given keys and lock type
 	 *
 	 * @param array $paths List of paths
-	 * @param $type integer LockManager::LOCK_* constant
+	 * @param int $type LockManager::LOCK_* constant
 	 * @return Status
 	 */
 	abstract protected function doUnlock( array $paths, $type );
diff --git a/includes/filebackend/lockmanager/LockManagerGroup.php b/includes/filebackend/lockmanager/LockManagerGroup.php
index 9aff2415..19fc4fef 100644
--- a/includes/filebackend/lockmanager/LockManagerGroup.php
+++ b/includes/filebackend/lockmanager/LockManagerGroup.php
@@ -29,12 +29,12 @@
  * @since 1.19
  */
 class LockManagerGroup {
-	/** @var Array (domain => LockManager) */
+	/** @var array (domain => LockManager) */
 	protected static $instances = array();
 
 	protected $domain; // string; domain (usually wiki ID)
 
-	/** @var Array of (name => ('class' => ..., 'config' => ..., 'instance' => ...)) */
+	/** @var array Array of (name => ('class' => ..., 'config' => ..., 'instance' => ...)) */
 	protected $managers = array();
 
 	/**
@@ -45,7 +45,7 @@ class LockManagerGroup {
 	}
 
 	/**
-	 * @param string $domain Domain (usually wiki ID)
+	 * @param bool|string $domain Domain (usually wiki ID). Default: false.
 	 * @return LockManagerGroup
 	 */
 	public static function singleton( $domain = false ) {
@@ -54,13 +54,12 @@ class LockManagerGroup {
 			self::$instances[$domain] = new self( $domain );
 			self::$instances[$domain]->initFromGlobals();
 		}
+
 		return self::$instances[$domain];
 	}
 
 	/**
 	 * Destroy the singleton instances
-	 *
-	 * @return void
 	 */
 	public static function destroySingletons() {
 		self::$instances = array();
@@ -68,8 +67,6 @@ class LockManagerGroup {
 
 	/**
 	 * Register lock managers from the global variables
-	 *
-	 * @return void
 	 */
 	protected function initFromGlobals() {
 		global $wgLockManagers;
@@ -80,8 +77,7 @@ class LockManagerGroup {
 	/**
 	 * Register an array of file lock manager configurations
 	 *
-	 * @param $configs Array
-	 * @return void
+	 * @param array $configs
 	 * @throws MWException
 	 */
 	protected function register( array $configs ) {
@@ -107,7 +103,7 @@ class LockManagerGroup {
 	/**
 	 * Get the lock manager object with a given name
 	 *
-	 * @param $name string
+	 * @param string $name
 	 * @return LockManager
 	 * @throws MWException
 	 */
@@ -121,14 +117,15 @@ class LockManagerGroup {
 			$config = $this->managers[$name]['config'];
 			$this->managers[$name]['instance'] = new $class( $config );
 		}
+
 		return $this->managers[$name]['instance'];
 	}
 
 	/**
 	 * Get the config array for a lock manager object with a given name
 	 *
-	 * @param $name string
-	 * @return Array
+	 * @param string $name
+	 * @return array
 	 * @throws MWException
 	 */
 	public function config( $name ) {
@@ -136,6 +133,7 @@ class LockManagerGroup {
 			throw new MWException( "No lock manager defined with the name `$name`." );
 		}
 		$class = $this->managers[$name]['class'];
+
 		return array( 'class' => $class ) + $this->managers[$name]['config'];
 	}
 
diff --git a/includes/filebackend/lockmanager/MemcLockManager.php b/includes/filebackend/lockmanager/MemcLockManager.php
index 5eab03ee..9bb01c21 100644
--- a/includes/filebackend/lockmanager/MemcLockManager.php
+++ b/includes/filebackend/lockmanager/MemcLockManager.php
@@ -36,31 +36,31 @@
  * @since 1.20
  */
 class MemcLockManager extends QuorumLockManager {
-	/** @var Array Mapping of lock types to the type actually used */
+	/** @var array Mapping of lock types to the type actually used */
 	protected $lockTypeMap = array(
 		self::LOCK_SH => self::LOCK_SH,
 		self::LOCK_UW => self::LOCK_SH,
 		self::LOCK_EX => self::LOCK_EX
 	);
 
-	/** @var Array Map server names to MemcachedBagOStuff objects */
+	/** @var array Map server names to MemcachedBagOStuff objects */
 	protected $bagOStuffs = array();
-	/** @var Array */
-	protected $serversUp = array(); // (server name => bool)
 
-	protected $session = ''; // string; random UUID
+	/** @var array (server name => bool) */
+	protected $serversUp = array();
+
+	/** @var string Random UUID */
+	protected $session = '';
 
 	/**
 	 * Construct a new instance from configuration.
 	 *
-	 * $config paramaters include:
+	 * @param array $config Paramaters include:
 	 *   - lockServers  : Associative array of server names to "<IP>:<port>" strings.
 	 *   - srvsByBucket : Array of 1-16 consecutive integer keys, starting from 0,
 	 *                    each having an odd-numbered list of server names (peers) as values.
 	 *   - memcConfig   : Configuration array for ObjectCache::newFromParams. [optional]
 	 *                    If set, this must use one of the memcached classes.
-	 *
-	 * @param array $config
 	 * @throws MWException
 	 */
 	public function __construct( array $config ) {
@@ -88,7 +88,7 @@ class MemcLockManager extends QuorumLockManager {
 		$this->session = wfRandomString( 32 );
 	}
 
-	// @TODO: change this code to work in one batch
+	// @todo Change this code to work in one batch
 	protected function getLocksOnServer( $lockSrv, array $pathsByType ) {
 		$status = Status::newGood();
 
@@ -100,8 +100,8 @@ class MemcLockManager extends QuorumLockManager {
 					? array_merge( $lockedPaths[$type], $paths )
 					: $paths;
 			} else {
-				foreach ( $lockedPaths as $type => $paths ) {
-					$status->merge( $this->doFreeLocksOnServer( $lockSrv, $paths, $type ) );
+				foreach ( $lockedPaths as $lType => $lPaths ) {
+					$status->merge( $this->doFreeLocksOnServer( $lockSrv, $lPaths, $lType ) );
 				}
 				break;
 			}
@@ -110,7 +110,7 @@ class MemcLockManager extends QuorumLockManager {
 		return $status;
 	}
 
-	// @TODO: change this code to work in one batch
+	// @todo Change this code to work in one batch
 	protected function freeLocksOnServer( $lockSrv, array $pathsByType ) {
 		$status = Status::newGood();
 
@@ -123,6 +123,9 @@ class MemcLockManager extends QuorumLockManager {
 
 	/**
 	 * @see QuorumLockManager::getLocksOnServer()
+	 * @param string $lockSrv
+	 * @param array $paths
+	 * @param string $type
 	 * @return Status
 	 */
 	protected function doGetLocksOnServer( $lockSrv, array $paths, $type ) {
@@ -136,6 +139,7 @@ class MemcLockManager extends QuorumLockManager {
 			foreach ( $paths as $path ) {
 				$status->fatal( 'lockmanager-fail-acquirelock', $path );
 			}
+
 			return $status;
 		}
 
@@ -195,6 +199,9 @@ class MemcLockManager extends QuorumLockManager {
 
 	/**
 	 * @see QuorumLockManager::freeLocksOnServer()
+	 * @param string $lockSrv
+	 * @param array $paths
+	 * @param string $type
 	 * @return Status
 	 */
 	protected function doFreeLocksOnServer( $lockSrv, array $paths, $type ) {
@@ -208,7 +215,8 @@ class MemcLockManager extends QuorumLockManager {
 			foreach ( $paths as $path ) {
 				$status->fatal( 'lockmanager-fail-releaselock', $path );
 			}
-			return;
+
+			return $status;
 		}
 
 		// Fetch all the existing lock records...
@@ -254,6 +262,7 @@ class MemcLockManager extends QuorumLockManager {
 
 	/**
 	 * @see QuorumLockManager::isServerUp()
+	 * @param string $lockSrv
 	 * @return bool
 	 */
 	protected function isServerUp( $lockSrv ) {
@@ -280,11 +289,12 @@ class MemcLockManager extends QuorumLockManager {
 				return null; // server appears to be down
 			}
 		}
+
 		return $memc;
 	}
 
 	/**
-	 * @param $path string
+	 * @param string $path
 	 * @return string
 	 */
 	protected function recordKeyForPath( $path ) {
@@ -292,27 +302,28 @@ class MemcLockManager extends QuorumLockManager {
 	}
 
 	/**
-	 * @return Array An empty lock structure for a key
+	 * @return array An empty lock structure for a key
 	 */
 	protected static function newLockArray() {
 		return array( self::LOCK_SH => array(), self::LOCK_EX => array() );
 	}
 
 	/**
-	 * @param $a array
-	 * @return Array An empty lock structure for a key
+	 * @param array $a
+	 * @return array An empty lock structure for a key
 	 */
 	protected static function sanitizeLockArray( $a ) {
 		if ( is_array( $a ) && isset( $a[self::LOCK_EX] ) && isset( $a[self::LOCK_SH] ) ) {
 			return $a;
 		} else {
 			trigger_error( __METHOD__ . ": reset invalid lock array.", E_USER_WARNING );
+
 			return self::newLockArray();
 		}
 	}
 
 	/**
-	 * @param $memc MemcachedBagOStuff
+	 * @param MemcachedBagOStuff $memc
 	 * @param array $keys List of keys to acquire
 	 * @return bool
 	 */
@@ -350,9 +361,8 @@ class MemcLockManager extends QuorumLockManager {
 	}
 
 	/**
-	 * @param $memc MemcachedBagOStuff
+	 * @param MemcachedBagOStuff $memc
 	 * @param array $keys List of acquired keys
-	 * @return void
 	 */
 	protected function releaseMutexes( MemcachedBagOStuff $memc, array $keys ) {
 		foreach ( $keys as $key ) {
diff --git a/includes/filebackend/lockmanager/QuorumLockManager.php b/includes/filebackend/lockmanager/QuorumLockManager.php
index 8356d32a..a692012d 100644
--- a/includes/filebackend/lockmanager/QuorumLockManager.php
+++ b/includes/filebackend/lockmanager/QuorumLockManager.php
@@ -29,9 +29,10 @@
  * @since 1.20
  */
 abstract class QuorumLockManager extends LockManager {
-	/** @var Array Map of bucket indexes to peer server lists */
+	/** @var array Map of bucket indexes to peer server lists */
 	protected $srvsByBucket = array(); // (bucket index => (lsrv1, lsrv2, ...))
-	/** @var Array Map of degraded buckets */
+
+	/** @var array Map of degraded buckets */
 	protected $degradedBuckets = array(); // (buckey index => UNIX timestamp)
 
 	final protected function doLock( array $paths, $type ) {
@@ -65,6 +66,7 @@ abstract class QuorumLockManager extends LockManager {
 			$status->merge( $this->doLockingRequestBucket( $bucket, $pathsToLockByType ) );
 			if ( !$status->isOK() ) {
 				$status->merge( $this->doUnlockByType( $lockedPaths ) );
+
 				return $status;
 			}
 			// Record these locks as active
@@ -120,7 +122,7 @@ abstract class QuorumLockManager extends LockManager {
 	 * Attempt to acquire locks with the peers for a bucket.
 	 * This is all or nothing; if any key is locked then this totally fails.
 	 *
-	 * @param $bucket integer
+	 * @param int $bucket
 	 * @param array $pathsByType Map of LockManager::LOCK_* constants to lists of paths
 	 * @return Status
 	 */
@@ -162,7 +164,7 @@ abstract class QuorumLockManager extends LockManager {
 	/**
 	 * Attempt to release locks with the peers for a bucket
 	 *
-	 * @param $bucket integer
+	 * @param int $bucket
 	 * @param array $pathsByType Map of LockManager::LOCK_* constants to lists of paths
 	 * @return Status
 	 */
@@ -176,8 +178,8 @@ abstract class QuorumLockManager extends LockManager {
 		foreach ( $this->srvsByBucket[$bucket] as $lockSrv ) {
 			if ( !$this->isServerUp( $lockSrv ) ) {
 				$status->warning( 'lockmanager-fail-svr-release', $lockSrv );
-			// Attempt to release the lock on this peer
 			} else {
+				// Attempt to release the lock on this peer
 				$status->merge( $this->freeLocksOnServer( $lockSrv, $pathsByType ) );
 				++$yesVotes; // success for this peer
 				// Normally the first peers form the quorum, and the others are ignored.
@@ -198,8 +200,8 @@ abstract class QuorumLockManager extends LockManager {
 	 * Get the bucket for resource path.
 	 * This should avoid throwing any exceptions.
 	 *
-	 * @param $path string
-	 * @return integer
+	 * @param string $path
+	 * @return int
 	 */
 	protected function getBucketFromPath( $path ) {
 		$prefix = substr( sha1( $path ), 0, 2 ); // first 2 hex chars (8 bits)
@@ -210,7 +212,7 @@ abstract class QuorumLockManager extends LockManager {
 	 * Check if a lock server is up.
 	 * This should process cache results to reduce RTT.
 	 *
-	 * @param $lockSrv string
+	 * @param string $lockSrv
 	 * @return bool
 	 */
 	abstract protected function isServerUp( $lockSrv );
@@ -218,7 +220,7 @@ abstract class QuorumLockManager extends LockManager {
 	/**
 	 * Get a connection to a lock server and acquire locks
 	 *
-	 * @param $lockSrv string
+	 * @param string $lockSrv
 	 * @param array $pathsByType Map of LockManager::LOCK_* constants to lists of paths
 	 * @return Status
 	 */
@@ -229,7 +231,7 @@ abstract class QuorumLockManager extends LockManager {
 	 *
 	 * Subclasses must effectively implement this or releaseAllLocks().
 	 *
-	 * @param $lockSrv string
+	 * @param string $lockSrv
 	 * @param array $pathsByType Map of LockManager::LOCK_* constants to lists of paths
 	 * @return Status
 	 */
diff --git a/includes/filebackend/lockmanager/RedisLockManager.php b/includes/filebackend/lockmanager/RedisLockManager.php
index 43b0198a..90e05817 100644
--- a/includes/filebackend/lockmanager/RedisLockManager.php
+++ b/includes/filebackend/lockmanager/RedisLockManager.php
@@ -38,7 +38,7 @@
  * @since 1.22
  */
 class RedisLockManager extends QuorumLockManager {
-	/** @var Array Mapping of lock types to the type actually used */
+	/** @var array Mapping of lock types to the type actually used */
 	protected $lockTypeMap = array(
 		self::LOCK_SH => self::LOCK_SH,
 		self::LOCK_UW => self::LOCK_SH,
@@ -47,21 +47,21 @@ class RedisLockManager extends QuorumLockManager {
 
 	/** @var RedisConnectionPool */
 	protected $redisPool;
-	/** @var Array Map server names to hostname/IP and port numbers */
+
+	/** @var array Map server names to hostname/IP and port numbers */
 	protected $lockServers = array();
 
-	protected $session = ''; // string; random UUID
+	/** @var string Random UUID */
+	protected $session = '';
 
 	/**
 	 * Construct a new instance from configuration.
 	 *
-	 * $config paramaters include:
+	 * @param array $config Parameters include:
 	 *   - lockServers  : Associative array of server names to "<IP>:<port>" strings.
 	 *   - srvsByBucket : Array of 1-16 consecutive integer keys, starting from 0,
 	 *                    each having an odd-numbered list of server names (peers) as values.
 	 *   - redisConfig  : Configuration for RedisConnectionPool::__construct().
-	 *
-	 * @param Array $config
 	 * @throws MWException
 	 */
 	public function __construct( array $config ) {
@@ -78,115 +78,89 @@ class RedisLockManager extends QuorumLockManager {
 		$this->session = wfRandomString( 32 );
 	}
 
-	// @TODO: change this code to work in one batch
 	protected function getLocksOnServer( $lockSrv, array $pathsByType ) {
 		$status = Status::newGood();
 
-		$lockedPaths = array();
-		foreach ( $pathsByType as $type => $paths ) {
-			$status->merge( $this->doGetLocksOnServer( $lockSrv, $paths, $type ) );
-			if ( $status->isOK() ) {
-				$lockedPaths[$type] = isset( $lockedPaths[$type] )
-					? array_merge( $lockedPaths[$type], $paths )
-					: $paths;
-			} else {
-				foreach ( $lockedPaths as $type => $paths ) {
-					$status->merge( $this->doFreeLocksOnServer( $lockSrv, $paths, $type ) );
-				}
-				break;
-			}
-		}
-
-		return $status;
-	}
-
-	// @TODO: change this code to work in one batch
-	protected function freeLocksOnServer( $lockSrv, array $pathsByType ) {
-		$status = Status::newGood();
-
-		foreach ( $pathsByType as $type => $paths ) {
-			$status->merge( $this->doFreeLocksOnServer( $lockSrv, $paths, $type ) );
-		}
-
-		return $status;
-	}
-
-	protected function doGetLocksOnServer( $lockSrv, array $paths, $type ) {
-		$status = Status::newGood();
-
 		$server = $this->lockServers[$lockSrv];
 		$conn = $this->redisPool->getConnection( $server );
 		if ( !$conn ) {
-			foreach ( $paths as $path ) {
+			foreach ( array_merge( array_values( $pathsByType ) ) as $path ) {
 				$status->fatal( 'lockmanager-fail-acquirelock', $path );
 			}
+
 			return $status;
 		}
 
-		$keys = array_map( array( $this, 'recordKeyForPath' ), $paths ); // lock records
+		$pathsByKey = array(); // (type:hash => path) map
+		foreach ( $pathsByType as $type => $paths ) {
+			$typeString = ( $type == LockManager::LOCK_SH ) ? 'SH' : 'EX';
+			foreach ( $paths as $path ) {
+				$pathsByKey[$this->recordKeyForPath( $path, $typeString )] = $path;
+			}
+		}
 
 		try {
 			static $script =
 <<<LUA
-			if ARGV[1] ~= 'EX' and ARGV[1] ~= 'SH' then
-				return redis.error_reply('Unrecognized lock type given (must be EX or SH)')
-			end
 			local failed = {}
+			-- Load input params (e.g. session, ttl, time of request)
+			local rSession, rTTL, rTime = unpack(ARGV)
 			-- Check that all the locks can be acquired
-			for i,resourceKey in ipairs(KEYS) do
+			for i,requestKey in ipairs(KEYS) do
+				local _, _, rType, resourceKey = string.find(requestKey,"(%w+):(%w+)$")
 				local keyIsFree = true
 				local currentLocks = redis.call('hKeys',resourceKey)
 				for i,lockKey in ipairs(currentLocks) do
+					-- Get the type and session of this lock
 					local _, _, type, session = string.find(lockKey,"(%w+):(%w+)")
 					-- Check any locks that are not owned by this session
-					if session ~= ARGV[2] then
-						local lockTimestamp = redis.call('hGet',resourceKey,lockKey)
-						if 1*lockTimestamp < ( ARGV[4] - ARGV[3] ) then
+					if session ~= rSession then
+						local lockExpiry = redis.call('hGet',resourceKey,lockKey)
+						if 1*lockExpiry < 1*rTime then
 							-- Lock is stale, so just prune it out
 							redis.call('hDel',resourceKey,lockKey)
-						elseif ARGV[1] == 'EX' or type == 'EX' then
+						elseif rType == 'EX' or type == 'EX' then
 							keyIsFree = false
 							break
 						end
 					end
 				end
 				if not keyIsFree then
-					failed[#failed+1] = resourceKey
+					failed[#failed+1] = requestKey
 				end
 			end
 			-- If all locks could be acquired, then do so
 			if #failed == 0 then
-				for i,resourceKey in ipairs(KEYS) do
-					redis.call('hSet',resourceKey,ARGV[1] .. ':' .. ARGV[2],ARGV[4])
+				for i,requestKey in ipairs(KEYS) do
+					local _, _, rType, resourceKey = string.find(requestKey,"(%w+):(%w+)$")
+					redis.call('hSet',resourceKey,rType .. ':' .. rSession,rTime + rTTL)
 					-- In addition to invalidation logic, be sure to garbage collect
-					redis.call('expire',resourceKey,ARGV[3])
+					redis.call('expire',resourceKey,rTTL)
 				end
 			end
 			return failed
 LUA;
 			$res = $conn->luaEval( $script,
 				array_merge(
-					$keys, // KEYS[0], KEYS[1],...KEYS[N]
+					array_keys( $pathsByKey ), // KEYS[0], KEYS[1],...,KEYS[N]
 					array(
-						$type === self::LOCK_SH ? 'SH' : 'EX', // ARGV[1]
-						$this->session, // ARGV[2]
-						$this->lockTTL, // ARGV[3]
-						time() // ARGV[4]
+						$this->session, // ARGV[1]
+						$this->lockTTL, // ARGV[2]
+						time() // ARGV[3]
 					)
 				),
-				count( $keys ) # number of first argument(s) that are keys
+				count( $pathsByKey ) # number of first argument(s) that are keys
 			);
 		} catch ( RedisException $e ) {
 			$res = false;
-			$this->redisPool->handleException( $server, $conn, $e );
+			$this->redisPool->handleError( $conn, $e );
 		}
 
 		if ( $res === false ) {
-			foreach ( $paths as $path ) {
+			foreach ( array_merge( array_values( $pathsByType ) ) as $path ) {
 				$status->fatal( 'lockmanager-fail-acquirelock', $path );
 			}
 		} else {
-			$pathsByKey = array_combine( $keys, $paths );
 			foreach ( $res as $key ) {
 				$status->fatal( 'lockmanager-fail-acquirelock', $pathsByKey[$key] );
 			}
@@ -195,61 +169,66 @@ LUA;
 		return $status;
 	}
 
-	protected function doFreeLocksOnServer( $lockSrv, array $paths, $type ) {
+	protected function freeLocksOnServer( $lockSrv, array $pathsByType ) {
 		$status = Status::newGood();
 
 		$server = $this->lockServers[$lockSrv];
 		$conn = $this->redisPool->getConnection( $server );
 		if ( !$conn ) {
-			foreach ( $paths as $path ) {
+			foreach ( array_merge( array_values( $pathsByType ) ) as $path ) {
 				$status->fatal( 'lockmanager-fail-releaselock', $path );
 			}
+
 			return $status;
 		}
 
-		$keys = array_map( array( $this, 'recordKeyForPath' ), $paths ); // lock records
+		$pathsByKey = array(); // (type:hash => path) map
+		foreach ( $pathsByType as $type => $paths ) {
+			$typeString = ( $type == LockManager::LOCK_SH ) ? 'SH' : 'EX';
+			foreach ( $paths as $path ) {
+				$pathsByKey[$this->recordKeyForPath( $path, $typeString )] = $path;
+			}
+		}
 
 		try {
 			static $script =
 <<<LUA
-			if ARGV[1] ~= 'EX' and ARGV[1] ~= 'SH' then
-				return redis.error_reply('Unrecognized lock type given (must be EX or SH)')
-			end
 			local failed = {}
-			for i,resourceKey in ipairs(KEYS) do
-				local released = redis.call('hDel',resourceKey,ARGV[1] .. ':' .. ARGV[2])
+			-- Load input params (e.g. session)
+			local rSession = unpack(ARGV)
+			for i,requestKey in ipairs(KEYS) do
+				local _, _, rType, resourceKey = string.find(requestKey,"(%w+):(%w+)$")
+				local released = redis.call('hDel',resourceKey,rType .. ':' .. rSession)
 				if released > 0 then
 					-- Remove the whole structure if it is now empty
 					if redis.call('hLen',resourceKey) == 0 then
 						redis.call('del',resourceKey)
 					end
 				else
-					failed[#failed+1] = resourceKey
+					failed[#failed+1] = requestKey
 				end
 			end
 			return failed
 LUA;
 			$res = $conn->luaEval( $script,
 				array_merge(
-					$keys, // KEYS[0], KEYS[1],...KEYS[N]
+					array_keys( $pathsByKey ), // KEYS[0], KEYS[1],...,KEYS[N]
 					array(
-						$type === self::LOCK_SH ? 'SH' : 'EX', // ARGV[1]
-						$this->session // ARGV[2]
+						$this->session, // ARGV[1]
 					)
 				),
-				count( $keys ) # number of first argument(s) that are keys
+				count( $pathsByKey ) # number of first argument(s) that are keys
 			);
 		} catch ( RedisException $e ) {
 			$res = false;
-			$this->redisPool->handleException( $server, $conn, $e );
+			$this->redisPool->handleError( $conn, $e );
 		}
 
 		if ( $res === false ) {
-			foreach ( $paths as $path ) {
+			foreach ( array_merge( array_values( $pathsByType ) ) as $path ) {
 				$status->fatal( 'lockmanager-fail-releaselock', $path );
 			}
 		} else {
-			$pathsByKey = array_combine( $keys, $paths );
 			foreach ( $res as $key ) {
 				$status->fatal( 'lockmanager-fail-releaselock', $pathsByKey[$key] );
 			}
@@ -267,11 +246,13 @@ LUA;
 	}
 
 	/**
-	 * @param $path string
+	 * @param string $path
+	 * @param string $type One of (EX,SH)
 	 * @return string
 	 */
-	protected function recordKeyForPath( $path ) {
-		return implode( ':', array( __CLASS__, 'locks', $this->sha1Base36Absolute( $path ) ) );
+	protected function recordKeyForPath( $path, $type ) {
+		return implode( ':',
+			array( __CLASS__, 'locks', "$type:" . $this->sha1Base36Absolute( $path ) ) );
 	}
 
 	/**
@@ -279,10 +260,13 @@ LUA;
 	 */
 	function __destruct() {
 		while ( count( $this->locksHeld ) ) {
+			$pathsByType = array();
 			foreach ( $this->locksHeld as $path => $locks ) {
-				$this->doUnlock( array( $path ), self::LOCK_EX );
-				$this->doUnlock( array( $path ), self::LOCK_SH );
+				foreach ( $locks as $type => $count ) {
+					$pathsByType[$type][] = $path;
+				}
 			}
+			$this->unlockByType( $pathsByType );
 		}
 	}
 }
diff --git a/includes/filebackend/lockmanager/ScopedLock.php b/includes/filebackend/lockmanager/ScopedLock.php
index 5faad4a6..2056e101 100644
--- a/includes/filebackend/lockmanager/ScopedLock.php
+++ b/includes/filebackend/lockmanager/ScopedLock.php
@@ -34,9 +34,11 @@
 class ScopedLock {
 	/** @var LockManager */
 	protected $manager;
+
 	/** @var Status */
 	protected $status;
-	/** @var Array Map of lock types to resource paths */
+
+	/** @var array Map of lock types to resource paths */
 	protected $pathsByType;
 
 	/**
@@ -55,14 +57,13 @@ class ScopedLock {
 	 * Any locks are released once this object goes out of scope.
 	 * The status object is updated with any errors or warnings.
 	 *
-	 * $type can be "mixed" and $paths can be a map of types to paths (since 1.22).
-	 * Otherwise $type should be an integer and $paths should be a list of paths.
-	 *
 	 * @param LockManager $manager
 	 * @param array $paths List of storage paths or map of lock types to path lists
-	 * @param integer|string $type LockManager::LOCK_* constant or "mixed"
+	 * @param int|string $type LockManager::LOCK_* constant or "mixed" and $paths
+	 *   can be a map of types to paths (since 1.22). Otherwise $type should be an
+	 *   integer and $paths should be a list of paths.
 	 * @param Status $status
-	 * @param integer $timeout Timeout in seconds (0 means non-blocking) (since 1.22)
+	 * @param int $timeout Timeout in seconds (0 means non-blocking) (since 1.22)
 	 * @return ScopedLock|null Returns null on failure
 	 */
 	public static function factory(
@@ -74,6 +75,7 @@ class ScopedLock {
 		if ( $lockStatus->isOK() ) {
 			return new self( $manager, $pathsByType, $status );
 		}
+
 		return null;
 	}
 
@@ -83,7 +85,6 @@ class ScopedLock {
 	 * This is the same as setting the lock object to null.
 	 *
 	 * @param ScopedLock $lock
-	 * @return void
 	 * @since 1.21
 	 */
 	public static function release( ScopedLock &$lock = null ) {