filebackend: try to combine SH and EX lock acquisition

* getPathsToLockForOpsInternal() now returns an array in the format LockManager can use
* Also fixed getScopedLocksForOps() for multiwrite backend (it should accept proxy backend paths)
* Updated getScopedFileLocks() docs

Change-Id: Id0dc50c159c5fbc8fca10a9e46c5be23afcb3e9a

diff --git a/includes/filebackend/FileBackend.php b/includes/filebackend/FileBackend.php
index bdeb578..f586578 100644 (file)
--- a/includes/filebackend/FileBackend.php
+++ b/includes/filebackend/FileBackend.php
@@ -1183,8 +1183,10 @@ abstract class FileBackend {
         * Once the return value goes out scope, the locks will be released and
         * the status updated. Unlock fatals will not change the status "OK" value.
         *
-        * @param array $paths Storage paths
-        * @param integer $type LockManager::LOCK_* constant
+        * @see ScopedLock::factory()
+        *
+        * @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 Status to update on lock/unlock
         * @return ScopedLock|null Returns null on failure
         */

diff --git a/includes/filebackend/FileBackendMultiWrite.php b/includes/filebackend/FileBackendMultiWrite.php
index 7d35487..97584a7 100644 (file)
--- a/includes/filebackend/FileBackendMultiWrite.php
+++ b/includes/filebackend/FileBackendMultiWrite.php
@@ -141,17 +141,10 @@ class FileBackendMultiWrite extends FileBackend {
 
                $mbe = $this->backends[$this->masterIndex]; // convenience
 
-               // Get the paths to lock from the master backend
-               $realOps = $this->substOpBatchPaths( $ops, $mbe );
-               $paths = $mbe->getPathsToLockForOpsInternal( $mbe->getOperationsInternal( $realOps ) );
-               // Get the paths under the proxy backend's name
-               $paths['sh'] = $this->unsubstPaths( $paths['sh'] );
-               $paths['ex'] = $this->unsubstPaths( $paths['ex'] );
                // Try to lock those files for the scope of this function...
                if ( empty( $opts['nonLocking'] ) ) {
                        // Try to lock those files for the scope of this function...
-                       $scopeLockS = $this->getScopedFileLocks( $paths['sh'], LockManager::LOCK_UW, $status );
-                       $scopeLockE = $this->getScopedFileLocks( $paths['ex'], LockManager::LOCK_EX, $status );
+                       $scopeLock = $this->getScopedLocksForOps( $ops, $status );
                        if ( !$status->isOK() ) {
                                return $status; // abort
                        }
@@ -178,6 +171,7 @@ class FileBackendMultiWrite extends FileBackend {
                        }
                }
                // Actually attempt the operation batch on the master backend...
+               $realOps = $this->substOpBatchPaths( $ops, $mbe );
                $masterStatus = $mbe->doOperations( $realOps, $opts );
                $status->merge( $masterStatus );
                // Propagate the operations to the clone backends if there were no unexpected errors
@@ -624,15 +618,16 @@ class FileBackendMultiWrite extends FileBackend {
        }
 
        public function getScopedLocksForOps( array $ops, Status $status ) {
-               $fileOps = $this->backends[$this->masterIndex]->getOperationsInternal( $ops );
+               $realOps = $this->substOpBatchPaths( $ops, $this->backends[$this->masterIndex] );
+               $fileOps = $this->backends[$this->masterIndex]->getOperationsInternal( $realOps );
                // Get the paths to lock from the master backend
                $paths = $this->backends[$this->masterIndex]->getPathsToLockForOpsInternal( $fileOps );
                // Get the paths under the proxy backend's name
-               $paths['sh'] = $this->unsubstPaths( $paths['sh'] );
-               $paths['ex'] = $this->unsubstPaths( $paths['ex'] );
-               return array(
-                       $this->getScopedFileLocks( $paths['sh'], LockManager::LOCK_UW, $status ),
-                       $this->getScopedFileLocks( $paths['ex'], LockManager::LOCK_EX, $status )
+               $pbPaths = array(
+                       LockManager::LOCK_UW => $this->unsubstPaths( $paths[LockManager::LOCK_UW] ),
+                       LockManager::LOCK_EX => $this->unsubstPaths( $paths[LockManager::LOCK_EX] )
                );
+               // Actually acquire the locks
+               return array( $this->getScopedFileLocks( $pbPaths, 'mixed', $status ) );
        }
 }

diff --git a/includes/filebackend/FileBackendStore.php b/includes/filebackend/FileBackendStore.php
index 8ff383b..0921e99 100644 (file)
--- a/includes/filebackend/FileBackendStore.php
+++ b/includes/filebackend/FileBackendStore.php
@@ -953,12 +953,13 @@ abstract class FileBackendStore extends FileBackend {
 
        /**
         * Get a list of storage paths to lock for a list of operations
-        * Returns an array with 'sh' (shared) and 'ex' (exclusive) keys,
-        * each corresponding to a list of storage paths to be locked.
-        * All returned paths are normalized.
+        * Returns an array with LockManager::LOCK_UW (shared locks) and
+        * LockManager::LOCK_EX (exclusive locks) keys, each corresponding
+        * to a list of storage paths to be locked. All returned paths are
+        * normalized.
         *
         * @param array $performOps List of FileOp objects
-        * @return Array ('sh' => list of paths, 'ex' => list of paths)
+        * @return Array (LockManager::LOCK_UW => path list, LockManager::LOCK_EX => path list)
         */
        final public function getPathsToLockForOpsInternal( array $performOps ) {
                // Build up a list of files to lock...
@@ -972,15 +973,15 @@ abstract class FileBackendStore extends FileBackend {
                // Get a shared lock on the parent directory of each path changed
                $paths['sh'] = array_merge( $paths['sh'], array_map( 'dirname', $paths['ex'] ) );
 
-               return $paths;
+               return array(
+                       LockManager::LOCK_UW => $paths['sh'],
+                       LockManager::LOCK_EX => $paths['ex']
+               );
        }
 
        public function getScopedLocksForOps( array $ops, Status $status ) {
                $paths = $this->getPathsToLockForOpsInternal( $this->getOperationsInternal( $ops ) );
-               return array(
-                       $this->getScopedFileLocks( $paths['sh'], LockManager::LOCK_UW, $status ),
-                       $this->getScopedFileLocks( $paths['ex'], LockManager::LOCK_EX, $status )
-               );
+               return array( $this->getScopedFileLocks( $paths, 'mixed', $status ) );
        }
 
        final protected function doOperationsInternal( array $ops, array $opts ) {
@@ -998,8 +999,7 @@ abstract class FileBackendStore extends FileBackend {
                        // Build up a list of files to lock...
                        $paths = $this->getPathsToLockForOpsInternal( $performOps );
                        // Try to lock those files for the scope of this function...
-                       $scopeLockS = $this->getScopedFileLocks( $paths['sh'], LockManager::LOCK_UW, $status );
-                       $scopeLockE = $this->getScopedFileLocks( $paths['ex'], LockManager::LOCK_EX, $status );
+                       $scopeLock = $this->getScopedFileLocks( $paths, 'mixed', $status );
                        if ( !$status->isOK() ) {
                                return $status; // abort
                        }