[LockManager] Made it easier to get both SH and EX locks at once.

* This also makes it possible for subclasses to optimize this case.
* Added a timeout parameter to ScopedLock::factory().
* Cleaned up a few bits of documentation.

Change-Id: Id3e9cf01f25ab498ea00d87ffb6d00aa8b05052b

diff --git a/includes/filebackend/lockmanager/LockManager.php b/includes/filebackend/lockmanager/LockManager.php
index 9c778c7..f0e54ec 100644 (file)
--- a/includes/filebackend/lockmanager/LockManager.php
+++ b/includes/filebackend/lockmanager/LockManager.php
@@ -56,7 +56,7 @@ abstract class LockManager {
        protected $domain; // string; domain (usually wiki ID)
        protected $lockTTL; // integer; maximum time locks can be held
 
-       /* Lock types; stronger locks have higher values */
+       /** Lock types; stronger locks have higher values */
        const LOCK_SH = 1; // shared lock (for reads)
        const LOCK_UW = 2; // shared lock (for reads used to write elsewhere)
        const LOCK_EX = 3; // exclusive lock (for writes)
@@ -92,11 +92,25 @@ abstract class LockManager {
         * @return Status
         */
        final public function lock( array $paths, $type = self::LOCK_EX, $timeout = 0 ) {
+               return $this->lockByType( array( $type => $paths ), $timeout );
+       }
+
+       /**
+        * Lock the resources at the given abstract paths
+        *
+        * @param array $paths Map of LockManager::LOCK_* constants to lists of storage paths
+        * @param integer $timeout Timeout in seconds (0 means non-blocking) (since 1.21)
+        * @return Status
+        * @since 1.22
+        */
+       final public function lockByType( array $pathsByType, $timeout = 0 ) {
                wfProfileIn( __METHOD__ );
+               $status = Status::newGood();
+               $pathsByType = $this->normalizePathsByType( $pathsByType );
                $msleep = array( 0, 50, 100, 300, 500 ); // retry backoff times
                $start = microtime( true );
                do {
-                       $status = $this->doLock( array_unique( $paths ), $this->lockTypeMap[$type] );
+                       $status = $this->doLockByType( $pathsByType );
                        $elapsed = microtime( true ) - $start;
                        if ( $status->isOK() || $elapsed >= $timeout || $elapsed < 0 ) {
                                break; // success, timeout, or clock set back
@@ -116,8 +130,20 @@ abstract class LockManager {
         * @return Status
         */
        final public function unlock( array $paths, $type = self::LOCK_EX ) {
+               return $this->unlockByType( array( $type => $paths ) );
+       }
+
+       /**
+        * Unlock the resources at the given abstract paths
+        *
+        * @param array $paths Map of LockManager::LOCK_* constants to lists of storage paths
+        * @return Status
+        * @since 1.22
+        */
+       final public function unlockByType( array $pathsByType ) {
                wfProfileIn( __METHOD__ );
-               $status = $this->doUnlock( array_unique( $paths ), $this->lockTypeMap[$type] );
+               $pathsByType = $this->normalizePathsByType( $pathsByType );
+               $status = $this->doUnlockByType( $pathsByType );
                wfProfileOut( __METHOD__ );
                return $status;
        }
@@ -146,6 +172,46 @@ abstract class LockManager {
                return sha1( "{$this->domain}:{$path}" );
        }
 
+       /**
+        * 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 storage paths
+        * @return Array
+        * @since 1.22
+        */
+       final protected function normalizePathsByType( array $pathsByType ) {
+               $res = array();
+               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 storage paths
+        * @return Status
+        * @since 1.22
+        */
+       protected function doLockByType( array $pathsByType ) {
+               $status = Status::newGood();
+               $lockedByType = array(); // map of (type => paths)
+               foreach ( $pathsByType as $type => $paths ) {
+                       $status->merge( $this->doLock( $paths, $type ) );
+                       if ( $status->isOK() ) {
+                               $lockedByType[$type] = $paths;
+                       } else {
+                               // Release the subset of locks that were acquired
+                               foreach ( $lockedByType as $type => $paths ) {
+                                       $status->merge( $this->doUnlock( $paths, $type ) );
+                               }
+                               break;
+                       }
+               }
+               return $status;
+       }
+
        /**
         * Lock resources with the given keys and lock type
         *
@@ -155,6 +221,20 @@ abstract class LockManager {
         */
        abstract protected function doLock( array $paths, $type );
 
+       /**
+        * @see LockManager::unlockByType()
+        * @param array $paths Map of LockManager::LOCK_* constants to lists of storage paths
+        * @return Status
+        * @since 1.22
+        */
+       protected function doUnlockByType( array $pathsByType ) {
+               $status = Status::newGood();
+               foreach ( $pathsByType as $type => $paths ) {
+                       $status->merge( $this->doUnlock( $paths, $type ) );
+               }
+               return $status;
+       }
+
        /**
         * Unlock resources with the given keys and lock type
         *

diff --git a/includes/filebackend/lockmanager/ScopedLock.php b/includes/filebackend/lockmanager/ScopedLock.php
index edcb1d6..5faad4a 100644 (file)
--- a/includes/filebackend/lockmanager/ScopedLock.php
+++ b/includes/filebackend/lockmanager/ScopedLock.php
@@ -36,24 +36,18 @@ class ScopedLock {
        protected $manager;
        /** @var Status */
        protected $status;
-       /** @var Array List of resource paths*/
-       protected $paths;
-
-       protected $type; // integer lock type
+       /** @var Array Map of lock types to resource paths */
+       protected $pathsByType;
 
        /**
-        * @param $manager LockManager
-        * @param array $paths List of storage paths
-        * @param $type integer LockManager::LOCK_* constant
-        * @param $status Status
+        * @param LockManager $manager
+        * @param array $pathsByType Map of lock types to path lists
+        * @param Status $status
         */
-       protected function __construct(
-               LockManager $manager, array $paths, $type, Status $status
-       ) {
+       protected function __construct( LockManager $manager, array $pathsByType, Status $status ) {
                $this->manager = $manager;
-               $this->paths = $paths;
+               $this->pathsByType = $pathsByType;
                $this->status = $status;
-               $this->type = $type;
        }
 
        /**
@@ -61,19 +55,24 @@ class ScopedLock {
         * Any locks are released once this object goes out of scope.
         * The status object is updated with any errors or warnings.
         *
-        * @param $manager LockManager
-        * @param array $paths List of storage paths
-        * @param $type integer LockManager::LOCK_* constant
-        * @param $status Status
+        * $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 Status $status
+        * @param integer $timeout Timeout in seconds (0 means non-blocking) (since 1.22)
         * @return ScopedLock|null Returns null on failure
         */
        public static function factory(
-               LockManager $manager, array $paths, $type, Status $status
+               LockManager $manager, array $paths, $type, Status $status, $timeout = 0
        ) {
-               $lockStatus = $manager->lock( $paths, $type );
+               $pathsByType = is_integer( $type ) ? array( $type => $paths ) : $paths;
+               $lockStatus = $manager->lockByType( $pathsByType, $timeout );
                $status->merge( $lockStatus );
                if ( $lockStatus->isOK() ) {
-                       return new self( $manager, $paths, $type, $status );
+                       return new self( $manager, $pathsByType, $status );
                }
                return null;
        }
@@ -91,9 +90,12 @@ class ScopedLock {
                $lock = null;
        }
 
+       /**
+        * Release the locks when this goes out of scope
+        */
        function __destruct() {
                $wasOk = $this->status->isOK();
-               $this->status->merge( $this->manager->unlock( $this->paths, $this->type ) );
+               $this->status->merge( $this->manager->unlockByType( $this->pathsByType ) );
                if ( $wasOk ) {
                        // Make sure status is OK, despite any unlockFiles() fatals
                        $this->status->setResult( true, $this->status->value );