Fixed bug where catching DB errors left ignoreErrors() on
authorAaron Schulz <aschulz@wikimedia.org>
Thu, 19 Mar 2015 18:42:48 +0000 (11:42 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Tue, 7 Apr 2015 18:12:06 +0000 (18:12 +0000)
* Also fixed an actual rollback loop possible in postgres

Change-Id: I41508127f74e1bbee4c020546fed85ab53318ab7

includes/db/Database.php
includes/db/DatabasePostgres.php

index cbd8be5..0c0248d 100644 (file)
@@ -1226,13 +1226,10 @@ abstract class DatabaseBase implements IDatabase {
         * @throws DBQueryError
         */
        public function reportQueryError( $error, $errno, $sql, $fname, $tempIgnore = false ) {
-               # Ignore errors during error handling to avoid infinite recursion
-               $ignore = $this->ignoreErrors( true );
                ++$this->mErrorCount;
 
-               if ( $ignore || $tempIgnore ) {
+               if ( $this->ignoreErrors() || $tempIgnore ) {
                        wfDebug( "SQL ERROR (ignored): $error\n" );
-                       $this->ignoreErrors( $ignore );
                } else {
                        $sql1line = mb_substr( str_replace( "\n", "\\n", $sql ), 0, 5 * 1024 );
                        wfLogDBError(
@@ -3324,41 +3321,40 @@ abstract class DatabaseBase implements IDatabase {
         * @return bool
         */
        public function deadlockLoop() {
-               $this->begin( __METHOD__ );
                $args = func_get_args();
                $function = array_shift( $args );
-               $oldIgnore = $this->ignoreErrors( true );
                $tries = self::DEADLOCK_TRIES;
-
                if ( is_array( $function ) ) {
                        $fname = $function[0];
                } else {
                        $fname = $function;
                }
 
-               do {
-                       $retVal = call_user_func_array( $function, $args );
-                       $error = $this->lastError();
-                       $errno = $this->lastErrno();
-                       $sql = $this->lastQuery();
+               $this->begin( __METHOD__ );
 
-                       if ( $errno ) {
+               $e = null;
+               do {
+                       try {
+                               $retVal = call_user_func_array( $function, $args );
+                               break;
+                       } catch ( DBQueryError $e ) {
+                               $error = $this->lastError();
+                               $errno = $this->lastErrno();
+                               $sql = $this->lastQuery();
                                if ( $this->wasDeadlock() ) {
-                                       # Retry
+                                       // Retry after a randomized delay
                                        usleep( mt_rand( self::DEADLOCK_DELAY_MIN, self::DEADLOCK_DELAY_MAX ) );
                                } else {
-                                       $this->reportQueryError( $error, $errno, $sql, $fname );
+                                       // Throw the error back up
+                                       throw $e;
                                }
                        }
-               } while ( $this->wasDeadlock() && --$tries > 0 );
-
-               $this->ignoreErrors( $oldIgnore );
+               } while ( --$tries > 0 );
 
                if ( $tries <= 0 ) {
+                       // Too many deadlocks; give up
                        $this->rollback( __METHOD__ );
-                       $this->reportQueryError( $error, $errno, $sql, $fname );
-
-                       return false;
+                       throw $e;
                } else {
                        $this->commit( __METHOD__ );
 
index 3d0ed86..9287f7a 100644 (file)
@@ -530,10 +530,12 @@ class DatabasePostgres extends DatabaseBase {
                                return;
                        }
                }
-               /* Transaction stays in the ERROR state until rolledback */
+               /* Transaction stays in the ERROR state until rolled back */
                if ( $this->mTrxLevel ) {
+                       $ignore = $this->ignoreErrors( true );
                        $this->rollback( __METHOD__ );
-               };
+                       $this->ignoreErrors( $ignore );
+               }
                parent::reportQueryError( $error, $errno, $sql, $fname, false );
        }