From 8322771ec3cadaee965aa2d4cdb06ad274d4d9bd Mon Sep 17 00:00:00 2001
From: daniel <daniel.kinzler@wikimedia.de>
Date: Mon, 27 Aug 2012 14:46:23 +0200
Subject: [PATCH] Fix transaction nesting caused by LocalFile.

The transaction bracket in LocalFile::recordUpload2 used to span a call
to WikiPage::doEdit, which in turn opens a transaction. Nesting transactions
this way does not work: the first transaction is committed prematurely when
the second one starts. This may cause serious database corruption and
generally exciting behavior.

This change commits LocalFile's own transaction before any interaction
with WikiPage. There may be a race condition here, but that case seems to
be already handled in the code. Also, in the previous "broken" state, all
transactional protection got lost anyway - so this should at least be no
worse than what we had.

This may be changed back if and when we have support for nested
transactions.

Change-Id: I20d90fedb2d19c64ccf0f3942ddda756fe511c12
---
 includes/filerepo/file/LocalFile.php | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php
index 2630193e66..695c4e9eab 100644
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -1154,7 +1154,9 @@ class LocalFile extends File {
 		$logId = $log->addEntry( $action, $descTitle, $comment, array(), $user );
 
 		wfProfileIn( __METHOD__ . '-edit' );
-		if ( $descTitle->exists() ) {
+		$exists = $descTitle->exists();
+
+		if ( $exists ) {
 			# Create a null revision
 			$latest = $descTitle->getLatestRevID();
 			$nullRevision = Revision::newNullRevision(
@@ -1169,6 +1171,15 @@ class LocalFile extends File {
 				wfRunHooks( 'NewRevisionFromEditComplete', array( $wikiPage, $nullRevision, $latest, $user ) );
 				$wikiPage->updateRevisionOn( $dbw, $nullRevision );
 			}
+		}
+
+		# Commit the transaction now, in case something goes wrong later
+		# The most important thing is that files don't get lost, especially archives
+		# NOTE: once we have support for nested transactions, the commit may be moved
+		#       to after $wikiPage->doEdit has been called.
+		$dbw->commit( __METHOD__ );
+
+		if ( $exists ) {
 			# Invalidate the cache for the description page
 			$descTitle->invalidateCache();
 			$descTitle->purgeSquid();
@@ -1178,16 +1189,18 @@ class LocalFile extends File {
 			# Squid and file cache for the description page are purged by doEdit.
 			$status = $wikiPage->doEdit( $pageText, $comment, EDIT_NEW | EDIT_SUPPRESS_RC, false, $user );
 
-			if ( isset( $status->value['revision'] ) ) {
-				$dbw->update( 'logging', array( 'log_page' => $status->value['revision']->getPage() ), array( 'log_id' => $logId ), __METHOD__ );
+			if ( isset( $status->value['revision'] ) ) { // XXX; doEdit() uses a transaction
+				$dbw->begin();
+				$dbw->update( 'logging',
+					array( 'log_page' => $status->value['revision']->getPage() ),
+					array( 'log_id' => $logId ),
+					__METHOD__
+				);
+				$dbw->commit(); // commit before anything bad can happen
 			}
 		}
 		wfProfileOut( __METHOD__ . '-edit' );
 
-		# Commit the transaction now, in case something goes wrong later
-		# The most important thing is that files don't get lost, especially archives
-		$dbw->commit( __METHOD__ );
-
 		# Save to cache and purge the squid
 		# We shall not saveToCache before the commit since otherwise
 		# in case of a rollback there is an usable file from memcached
-- 
2.20.1