From 43aa35016b03935b27d439afe9a6b3f1aad1aa8b Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Sun, 6 May 2012 23:28:03 -0700
Subject: [PATCH] [FileRepo] Locking and transaction fixes.

* Make sure locks for file moves cover the whole operation.
* Pushed purging out of the move/delete/restore transactions.

Change-Id: I398c5627808fa79739f0dee632c4edf7416507c1
---
 includes/filerepo/file/LocalFile.php | 43 +++++++++++++---------------
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php
index 701a1ec2a3..68288e5234 100644
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -1231,19 +1231,18 @@ class LocalFile extends File {
 		}
 
 		wfDebugLog( 'imagemove', "Got request to move {$this->name} to " . $target->getText() );
-		$this->lock(); // begin
-
 		$batch = new LocalFileMoveBatch( $this, $target );
+
+		$this->lock(); // begin
 		$batch->addCurrent();
 		$batch->addOlds();
-
 		$status = $batch->execute();
+		$this->unlock(); // done
+
 		wfDebugLog( 'imagemove', "Finished moving {$this->name}" );
 
 		$this->purgeEverything();
-		$this->unlock(); // done
-
-		if ( $status->isOk() ) {
+		if ( $status->isOK() ) {
 			// Now switch the object
 			$this->title = $target;
 			// Force regeneration of the name and hashpath
@@ -1273,31 +1272,33 @@ class LocalFile extends File {
 			return $this->readOnlyFatalStatus();
 		}
 
-		$this->lock(); // begin
-
+		$dbw = $this->repo->getMasterDB();
 		$batch = new LocalFileDeleteBatch( $this, $reason, $suppress );
-		$batch->addCurrent();
+		$archiveNames = array();
 
+		$this->lock(); // begin
+		$batch->addCurrent();
 		# Get old version relative paths
-		$dbw = $this->repo->getMasterDB();
 		$result = $dbw->select( 'oldimage',
 			array( 'oi_archive_name' ),
 			array( 'oi_name' => $this->getName() ) );
 		foreach ( $result as $row ) {
 			$batch->addOld( $row->oi_archive_name );
-			$this->purgeOldThumbnails( $row->oi_archive_name );
+			$archiveNames[] = $row->oi_archive_name;
 		}
 		$status = $batch->execute();
-
 		if ( $status->isOK() ) {
 			// Update site_stats
 			$site_stats = $dbw->tableName( 'site_stats' );
 			$dbw->query( "UPDATE $site_stats SET ss_images=ss_images-1", __METHOD__ );
-			$this->purgeEverything();
 		}
-
 		$this->unlock(); // done
 
+		$this->purgeEverything();
+		foreach ( $archiveNames as $archiveName ) {
+			$this->purgeOldThumbnails( $archiveName );
+		}
+
 		return $status;
 	}
 
@@ -1353,25 +1354,21 @@ class LocalFile extends File {
 			return $this->readOnlyFatalStatus();
 		}
 
-		$this->lock(); // begin
-
 		$batch = new LocalFileRestoreBatch( $this, $unsuppress );
 
+		$this->lock(); // begin
 		if ( !$versions ) {
 			$batch->addAll();
 		} else {
 			$batch->addIds( $versions );
 		}
-
 		$status = $batch->execute();
-
 		if ( $status->isGood() ) {
 			$cleanupStatus = $batch->cleanup();
 			$cleanupStatus->successCount = 0;
 			$cleanupStatus->failCount = 0;
 			$status->merge( $cleanupStatus );
 		}
-
 		$this->unlock(); // done
 
 		return $status;
@@ -2171,7 +2168,7 @@ class LocalFileRestoreBatch {
 class LocalFileMoveBatch {
 
 	/**
-	 * @var File
+	 * @var LocalFile
 	 */
 	var $file;
 
@@ -2266,17 +2263,17 @@ class LocalFileMoveBatch {
 			return $statusMove;
 		}
 
-		$this->db->begin( __METHOD__ );
+		$this->file->lock(); // begin
 		$statusDb = $this->doDBUpdates();
 		wfDebugLog( 'imagemove', "Renamed {$this->file->getName()} in database: {$statusDb->successCount} successes, {$statusDb->failCount} failures" );
 		if ( !$statusDb->isGood() ) {
-			$this->db->rollback( __METHOD__ );
+			$this->file->unlockAndRollback();
 			// Something went wrong with the DB updates, so remove the target files
 			$this->cleanupTarget( $triplets );
 			$statusDb->ok = false;
 			return $statusDb;
 		}
-		$this->db->commit( __METHOD__ );
+		$this->file->unlock(); // done
 
 		// Everything went ok, remove the source files
 		$this->cleanupSource( $triplets );
-- 
2.20.1