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

diff --git a/includes/db/Database.php b/includes/db/Database.php
index a46f33d..5344b12 100644 (file)
--- 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
        }