Guard Database::explicitTrxActive() against returning false while in begin()

This can happen in the special case of the srvCache using callbacks
triggered by getApproximateLagStatus(). Treat the transaction as
explicit until begin() finishes to avoid this since it can cause
transaction nesting errors if caller thinks its OK to commit when
startAtomic() is still in progress.

Bug: T147697
Change-Id: If1c0f14f6a72cbcbee365afab8ee5a0872e746c8

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 9f1f228..fc55671 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -2682,7 +2682,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                $this->mTrxTimestamp = microtime( true );
                $this->mTrxFname = $fname;
                $this->mTrxDoneWrites = false;
-               $this->mTrxAutomatic = ( $mode === self::TRANSACTION_INTERNAL );
                $this->mTrxAutomaticAtomic = false;
                $this->mTrxAtomicLevels = [];
                $this->mTrxShortId = sprintf( '%06x', mt_rand( 0, 0xffffff ) );
@@ -2696,6 +2695,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                // as lag itself just to be safe
                $status = $this->getApproximateLagStatus();
                $this->mTrxReplicaLag = $status['lag'] + ( microtime( true ) - $status['since'] );
+               // T147697: make explicitTrxActive() return true until begin() finishes. This way, no
+               // caller will think its OK to muck around with the transaction just because startAtomic()
+               // has not yet completed (e.g. setting mTrxAtomicLevels).
+               $this->mTrxAutomatic = ( $mode === self::TRANSACTION_INTERNAL );
        }
 
        /**