From aed92ddf2abad37ef5bdbc3cd794ee906c5a77df Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Mon, 7 Dec 2015 17:26:15 -0800
Subject: [PATCH] Fix edge case in onTransactionIdle()

If a callback set DBO_TRX, make sure it gets unset if that
is not what the DB was set to before the callbacks ran.

Change-Id: I79b649de02e821494d7f88e8979764ec8a9d0c43
---
 includes/db/Database.php                   |  6 ++++-
 tests/phpunit/includes/db/DatabaseTest.php | 26 ++++++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/includes/db/Database.php b/includes/db/Database.php
index 68f94b6851..dea4a594f4 100644
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -3308,7 +3308,11 @@ abstract class DatabaseBase implements IDatabase {
 					list( $phpCallback ) = $callback;
 					$this->clearFlag( DBO_TRX ); // make each query its own transaction
 					call_user_func( $phpCallback );
-					$this->setFlag( $autoTrx ? DBO_TRX : 0 ); // restore automatic begin()
+					if ( $autoTrx ) {
+						$this->setFlag( DBO_TRX ); // restore automatic begin()
+					} else {
+						$this->clearFlag( DBO_TRX ); // restore auto-commit
+					}
 				} catch ( Exception $e ) {
 					if ( $ePrior ) {
 						MWExceptionHandler::logException( $ePrior );
diff --git a/tests/phpunit/includes/db/DatabaseTest.php b/tests/phpunit/includes/db/DatabaseTest.php
index 7e70439656..5f2a6fe970 100644
--- a/tests/phpunit/includes/db/DatabaseTest.php
+++ b/tests/phpunit/includes/db/DatabaseTest.php
@@ -234,4 +234,30 @@ class DatabaseTest extends MediaWikiTestCase {
 		$this->assertFalse( $this->db->tableExists( 'foobarbaz' ) );
 		$this->assertInternalType( 'int', $res->numRows() );
 	}
+
+	public function testTransactionIdle() {
+		$db = $this->db;
+
+		$db->setFlag( DBO_TRX );
+		$flagSet = null;
+		$db->onTransactionIdle( function() use ( $db, &$flagSet ) {
+			$flagSet = $db->getFlag( DBO_TRX );
+		} );
+		$this->assertFalse( $flagSet, 'DBO_TRX off in callback' );
+		$this->assertTrue( $db->getFlag( DBO_TRX ), 'DBO_TRX restored to default' );
+
+		$db->clearFlag( DBO_TRX );
+		$flagSet = null;
+		$db->onTransactionIdle( function() use ( $db, &$flagSet ) {
+			$flagSet = $db->getFlag( DBO_TRX );
+		} );
+		$this->assertFalse( $flagSet, 'DBO_TRX off in callback' );
+		$this->assertFalse( $db->getFlag( DBO_TRX ), 'DBO_TRX restored to default' );
+
+		$db->clearFlag( DBO_TRX );
+		$db->onTransactionIdle( function() use ( $db ) {
+			$db->setFlag( DBO_TRX );
+		} );
+		$this->assertFalse( $db->getFlag( DBO_TRX ), 'DBO_TRX restored to default' );
+	}
 }
-- 
2.20.1