rdbms: make runOnTransactionIdleCallbacks() reset DBO_TRX on exceptions
authorAaron Schulz <aschulz@wikimedia.org>
Fri, 25 May 2018 23:02:27 +0000 (16:02 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Fri, 25 May 2018 23:40:47 +0000 (23:40 +0000)
Change-Id: Ibbb2a3ebf9dd970772ee704aa643a3843f20a3b5

includes/libs/rdbms/database/Database.php
tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php

index aeda5b9..efef121 100644 (file)
@@ -3448,16 +3448,11 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        $this->trxIdleCallbacks = []; // consumed (and recursion guard)
                        $this->trxEndCallbacks = []; // consumed (recursion guard)
                        foreach ( $callbacks as $callback ) {
+                               ++$count;
+                               list( $phpCallback ) = $callback;
+                               $this->clearFlag( self::DBO_TRX ); // make each query its own transaction
                                try {
-                                       ++$count;
-                                       list( $phpCallback ) = $callback;
-                                       $this->clearFlag( self::DBO_TRX ); // make each query its own transaction
                                        call_user_func( $phpCallback, $trigger, $this );
-                                       if ( $autoTrx ) {
-                                               $this->setFlag( self::DBO_TRX ); // restore automatic begin()
-                                       } else {
-                                               $this->clearFlag( self::DBO_TRX ); // restore auto-commit
-                                       }
                                } catch ( Exception $ex ) {
                                        call_user_func( $this->errorLogger, $ex );
                                        $e = $e ?: $ex;
@@ -3466,6 +3461,12 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                                        if ( $this->trxLevel() ) {
                                                $this->rollback( __METHOD__, self::FLUSHING_INTERNAL );
                                        }
+                               } finally {
+                                       if ( $autoTrx ) {
+                                               $this->setFlag( self::DBO_TRX ); // restore automatic begin()
+                                       } else {
+                                               $this->clearFlag( self::DBO_TRX ); // restore auto-commit
+                                       }
                                }
                        }
                } while ( count( $this->trxIdleCallbacks ) );
index 3f0dae6..c12882b 100644 (file)
@@ -260,6 +260,16 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
 
                $lbFactory->commitMasterChanges( __METHOD__ );
                $this->assertFalse( $called, 'Not called in next round commit' );
+
+               $db->setFlag( DBO_TRX );
+               try {
+                       $db->onTransactionCommitOrIdle( function () {
+                               throw new RuntimeException( 'test' );
+                       } );
+                       $this->fail( "Exception not thrown" );
+               } catch ( RuntimeException $e ) {
+                       $this->assertTrue( $db->getFlag( DBO_TRX ) );
+               }
        }
 
        /**