From 5e53629c3c8fd9f0fa476df583e66b69e81ffe88 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 8 Mar 2018 12:40:07 -0800
Subject: [PATCH] rdbms: small cleanups to session loss handling

Split the two callback runner calls in handleSessionLoss() into two
separate try/catch loops. Return the first exception, if any.

Also make the $recoverable check for connection loss in Database::query
slightly more readable by checking the positive rather than the negative.

Change-Id: I75935fa69e40450ac3983f0d3451ab3001650b6f
---
 includes/libs/rdbms/database/Database.php | 28 +++++++++++++++--------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 2c59963c6d..014c4af399 100644
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -1015,12 +1015,12 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 				$this->queryLogger->warning( $msg, $params +
 					[ 'trace' => ( new RuntimeException() )->getTraceAsString() ] );
 
-				if ( !$recoverable ) {
-					# Callers may catch the exception and continue to use the DB
-					$this->reportQueryError( $lastError, $lastErrno, $sql, $fname );
-				} else {
+				if ( $recoverable ) {
 					# Should be safe to silently retry the query
 					$ret = $this->doProfiledQuery( $sql, $commentedSql, $isNonTempWrite, $fname );
+				} else {
+					# Callers may catch the exception and continue to use the DB
+					$this->reportQueryError( $lastError, $lastErrno, $sql, $fname );
 				}
 			} else {
 				$msg = __METHOD__ . ': lost connection to {dbserver} permanently';
@@ -1181,19 +1181,29 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 	 */
 	private function handleSessionLoss() {
 		$this->trxLevel = 0;
-		$this->trxIdleCallbacks = []; // T67263
-		$this->trxPreCommitCallbacks = []; // T67263
+		$this->trxIdleCallbacks = []; // T67263; transaction already lost
+		$this->trxPreCommitCallbacks = []; // T67263; transaction already lost
 		$this->sessionTempTables = [];
 		$this->namedLocksHeld = [];
+
+		// Note: if callback suppression is set then some *Callbacks arrays are not cleared here
+		$e = null;
 		try {
 			// Handle callbacks in trxEndCallbacks
 			$this->runOnTransactionIdleCallbacks( self::TRIGGER_ROLLBACK );
+		} catch ( Exception $ex ) {
+			// Already logged; move on...
+			$e = $e ?: $ex;
+		}
+		try {
+			// Handle callbacks in trxRecurringCallbacks
 			$this->runTransactionListenerCallbacks( self::TRIGGER_ROLLBACK );
-			return null;
-		} catch ( Exception $e ) {
+		} catch ( Exception $ex ) {
 			// Already logged; move on...
-			return $e;
+			$e = $e ?: $ex;
 		}
+
+		return $e;
 	}
 
 	/**
-- 
2.20.1