From 877c8afc3ebc66e7af53377317a009fcb3d132a9 Mon Sep 17 00:00:00 2001
From: daniel <daniel.kinzler@wikimedia.de>
Date: Mon, 27 Aug 2012 11:55:15 +0200
Subject: [PATCH] Trigger warning for bad use of transactions.

Mediawiki currently does not support nested transactions, which means that
starting a transaction while another is in progress will silently commit
the previous transaction, causing lots of potential inconsistency.

This change introduces checks that will log a warning whenever begin() is
used while a transaction is already in progress, and whenever commit() or
rollback() are used without a transaction being in progress.

NOTE: this exposes several places in the code where transactions have inadvertedly
be nested, or unmatched calls to begin() resp commit() are used. With
$wgDevelopmentWarnings enabled or I36583fb0 merged, this may cause tests to fail.
The following changes fix the issues in question: I80faf2ed, Ia225251e, Iff394f97,
and I20d90fed.

Change-Id: I8c0426e12b78edbd758b1f087c35dabd64498624
---
 includes/db/Database.php | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/includes/db/Database.php b/includes/db/Database.php
index a46f33d35d..5344b12fb2 100644
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -246,6 +246,15 @@ abstract class DatabaseBase implements DatabaseType {
 
 	protected $delimiter = ';';
 
+	/**
+	 * Remembers the function name given for starting the most recent transaction via the begin() method.
+	 * Used to provide additional context for error reporting.
+	 *
+	 * @var String
+	 * @see DatabaseBase::mTrxLevel
+	 */
+	private $mTrxFname = null;
+
 # ------------------------------------------------------------------------------
 # Accessors
 # ------------------------------------------------------------------------------
@@ -2875,10 +2884,13 @@ abstract class DatabaseBase implements DatabaseType {
 	 */
 	final public function begin( $fname = 'DatabaseBase::begin' ) {
 		if ( $this->mTrxLevel ) { // implicit commit
+			wfWarn( "$fname: Transaction already in progress (from {$this->mTrxFname}), " .
+				" performing implicit commit!" );
 			$this->doCommit( $fname );
 			$this->runOnTransactionIdleCallbacks();
 		}
 		$this->doBegin( $fname );
+		$this->mTrxFname = $fname;
 	}
 
 	/**
@@ -2896,6 +2908,9 @@ abstract class DatabaseBase implements DatabaseType {
 	 * @param $fname string
 	 */
 	final public function commit( $fname = 'DatabaseBase::commit' ) {
+		if ( !$this->mTrxLevel ) {
+			wfWarn( "$fname: No transaction to commit, something got out of sync!" );
+		}
 		$this->doCommit( $fname );
 		$this->runOnTransactionIdleCallbacks();
 	}
@@ -2918,6 +2933,9 @@ abstract class DatabaseBase implements DatabaseType {
 	 * @param $fname string
 	 */
 	final public function rollback( $fname = 'DatabaseBase::rollback' ) {
+		if ( !$this->mTrxLevel ) {
+			wfWarn( "$fname: No transaction to rollback, something got out of sync!" );
+		}
 		$this->doRollback( $fname );
 		$this->trxIdleCallbacks = array(); // cancel
 	}
-- 
2.20.1