From bede8b120bcb068eae4ccb77f8d3c4b10122c97b Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Mon, 29 Aug 2016 14:35:56 -0700
Subject: [PATCH] Disallow mismatched beginMasterChanges/commitMasterChanges

Change-Id: I76a8424e94370dc3776fdac1e974bf61fa69f071
---
 includes/db/loadbalancer/LBFactory.php | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/includes/db/loadbalancer/LBFactory.php b/includes/db/loadbalancer/LBFactory.php
index 6b5161b0b1..e40001dbae 100644
--- a/includes/db/loadbalancer/LBFactory.php
+++ b/includes/db/loadbalancer/LBFactory.php
@@ -230,6 +230,7 @@ abstract class LBFactory implements DestructibleService {
 	 *   - commitMasterChanges()
 	 *   - rollbackMasterChanges()
 	 *   - commitAll()
+	 *
 	 * This allows for custom transaction rounds from any outer transaction scope.
 	 *
 	 * @param string $fname
@@ -240,7 +241,7 @@ abstract class LBFactory implements DestructibleService {
 		if ( $this->trxRoundId !== false ) {
 			throw new DBTransactionError(
 				null,
-				"Transaction round '{$this->trxRoundId}' already started."
+				"$fname: transaction round '{$this->trxRoundId}' already started."
 			);
 		}
 		$this->trxRoundId = $fname;
@@ -279,6 +280,12 @@ abstract class LBFactory implements DestructibleService {
 	 * @throws Exception
 	 */
 	public function commitMasterChanges( $fname = __METHOD__, array $options = [] ) {
+		if ( $this->trxRoundId !== false && $this->trxRoundId !== $fname ) {
+			throw new DBTransactionError(
+				null,
+				"$fname: transaction round '{$this->trxRoundId}' still running."
+			);
+		}
 		// Run pre-commit callbacks and suppress post-commit callbacks, aborting on failure
 		$this->forEachLBCallMethod( 'finalizeMasterChanges' );
 		$this->trxRoundId = false;
@@ -509,7 +516,7 @@ abstract class LBFactory implements DestructibleService {
 	 * This will commit and wait unless $ticket indicates it is unsafe to do so
 	 *
 	 * @param string $fname Caller name (e.g. __METHOD__)
-	 * @param mixed $ticket Result of getOuterTransactionScopeTicket()
+	 * @param mixed $ticket Result of getEmptyTransactionTicket()
 	 * @param array $opts Options to waitForReplication()
 	 * @throws DBReplicationWaitError
 	 * @since 1.28
@@ -521,8 +528,21 @@ abstract class LBFactory implements DestructibleService {
 			return;
 		}
 
-		$this->commitMasterChanges( $fname );
+		$fnameEffective = $fname;
+		// The transaction owner and any caller with the empty transaction ticket can commit
+		// so that getEmptyTransactionTicket() callers don't risk seeing DBTransactionError.
+		if ( $this->trxRoundId !== false && $fname !== $this->trxRoundId ) {
+			$this->trxLogger->info( "$fname: committing on behalf of {$this->trxRoundId}." );
+			$fnameEffective = $this->trxRoundId;
+		}
+
+		$this->commitMasterChanges( $fnameEffective );
 		$this->waitForReplication( $opts );
+		// If a nested caller committed on behalf of $fname, start another empty $fname
+		// transaction, leaving the caller with the same empty transaction state as before.
+		if ( $fnameEffective !== $fname ) {
+			$this->beginMasterChanges( $fname );
+		}
 	}
 
 	/**
-- 
2.20.1