Detect when callers catch DB errors and fail to rollback

This makes it harder to accidently circumvent the MWExceptionHandler
rollback logic.

Change-Id: Ia1f89fa0f88ff3aacf5d9b93300dbf909fa74fdd

diff --git a/includes/db/DBConnRef.php b/includes/db/DBConnRef.php
index 4a78363..e5b6d05 100644 (file)
--- a/includes/db/DBConnRef.php
+++ b/includes/db/DBConnRef.php
@@ -55,6 +55,10 @@ class DBConnRef implements IDatabase {
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
+       public function explicitTrxActive() {
+               return $this->__call( __FUNCTION__, func_get_args() );
+       }
+
        public function tablePrefix( $prefix = null ) {
                return $this->__call( __FUNCTION__, func_get_args() );
        }

diff --git a/includes/db/Database.php b/includes/db/Database.php
index 933bea6..528a298 100644 (file)
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -2826,10 +2826,7 @@ abstract class DatabaseBase implements IDatabase {
                }
        }
 
-       /**
-        * @return bool
-        */
-       protected function explicitTrxActive() {
+       public function explicitTrxActive() {
                return $this->mTrxLevel && ( $this->mTrxAtomicLevels || !$this->mTrxAutomatic );
        }
 

diff --git a/includes/db/IDatabase.php b/includes/db/IDatabase.php
index bdab09e..25100db 100644 (file)
--- a/includes/db/IDatabase.php
+++ b/includes/db/IDatabase.php
@@ -103,6 +103,12 @@ interface IDatabase {
         */
        public function trxTimestamp();
 
+       /**
+        * @return bool Whether an explicit transaction or atomic sections are still open
+        * @since 1.28
+        */
+       public function explicitTrxActive();
+
        /**
         * Get/set the table prefix.
         * @param string $prefix The table prefix to set, or omitted to leave it unchanged.

diff --git a/includes/db/loadbalancer/LoadBalancer.php b/includes/db/loadbalancer/LoadBalancer.php
index d08172a..93ec037 100644 (file)
--- a/includes/db/loadbalancer/LoadBalancer.php
+++ b/includes/db/loadbalancer/LoadBalancer.php
@@ -1090,6 +1090,15 @@ class LoadBalancer {
        public function approveMasterChanges( array $options ) {
                $limit = isset( $options['maxWriteDuration'] ) ? $options['maxWriteDuration'] : 0;
                $this->forEachOpenMasterConnection( function ( DatabaseBase $conn ) use ( $limit ) {
+                       // If atomic section or explicit transactions are still open, some caller must have
+                       // caught an exception but failed to properly rollback any changes. Detect that and
+                       // throw and error (causing rollback).
+                       if ( $conn->explicitTrxActive() ) {
+                               throw new DBTransactionError(
+                                       $conn,
+                                       "Explicit transaction still active. A caller may have caught an error."
+                               );
+                       }
                        // Assert that the time to replicate the transaction will be sane.
                        // If this fails, then all DB transactions will be rollback back together.
                        $time = $conn->pendingWriteQueryDuration();