From 0ec0e192622a6df50830a86e27d07a8154d51c1e Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 20 Apr 2013 00:10:19 -0700 Subject: [PATCH] [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 --- .../filebackend/lockmanager/LockManager.php | 86 ++++++++++++++++++- .../filebackend/lockmanager/ScopedLock.php | 44 +++++----- 2 files changed, 106 insertions(+), 24 deletions(-) diff --git a/includes/filebackend/lockmanager/LockManager.php b/includes/filebackend/lockmanager/LockManager.php index 9c778c7e24..f0e54ec22f 100644 --- 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 edcb1d65cb..5faad4a644 100644 --- 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 ); -- 2.20.1