Improve connection handling and recovery in general, and specifically to address Issue #877:

 - On servers with very short timeouts set the wait_timeout for the session as well as the interactive_timeout to prevent the connection from dropping frequently
 - Improve recovery from connection errors, correctly restoring the connection if appropriate and possible
 - Allow reconnections to occur recursively by altering the internal tracking mechanism
 - Fix some edge cases where the connection would remain locked incorrectly
 - Improve error messaging for the "MySQL Server has gone away" network case

3 files changed, 29 insertions, 15 deletions

diff --git a/Frameworks/SPMySQLFramework/Source/SPMySQLConnection Categories/Querying & Preparation.m b/Frameworks/SPMySQLFramework/Source/SPMySQLConnection Categories/Querying & Preparation.m
index 3b2614f1..9280c156 100644
--- a/Frameworks/SPMySQLFramework/Source/SPMySQLConnection Categories/Querying & Preparation.m
+++ b/Frameworks/SPMySQLFramework/Source/SPMySQLConnection Categories/Querying & Preparation.m
@@ -217,6 +217,8 @@
 - (id)queryString:(NSString *)theQueryString usingEncoding:(NSStringEncoding)theEncoding withResultType:(SPMySQLResultType)theReturnType
 {
 	double queryExecutionTime;
+	NSString *theErrorMessage;
+	NSUInteger theErrorID;
 	lastQueryWasCancelled = NO;
 	lastQueryWasCancelledUsingReconnect = NO;
 
@@ -282,11 +284,17 @@
 
 		// If the query succeeded, no need to re-attempt.
 		if (!queryStatus) {
+			theErrorMessage = nil;
+			theErrorID = 0;
 			break;
 
 		// If the query failed, determine whether to reattempt the query
 		} else {
 
+			// Store the error state
+			theErrorMessage = [self _stringForCString:mysql_error(mySQLConnection)];
+			theErrorID = mysql_errno(mySQLConnection);
+
 			// Prevent retries if the query was cancelled or not a connection error
 			if (lastQueryWasCancelled || ![SPMySQLConnection isErrorIDConnectionError:mysql_errno(mySQLConnection)]) {
 				break;
@@ -294,10 +302,11 @@
 		}
 
 		// Query has failed - check the connection
+		[self _unlockConnection];
 		if (![self checkConnection]) {
-			[self _unlockConnection];
-			return nil;
+			break;
 		}
+		[self _lockConnection];
 
 		queryAttemptsAllowed--;
 	}
@@ -330,11 +339,11 @@
 				theResult = [[SPMySQLFastStreamingResult alloc] initWithMySQLResult:mysqlResult stringEncoding:theEncoding connection:self];
 				break;
 		}
-	}
 
-	// Record the error state now, as it may be affected by subsequent clean-up queries
-	NSString *theErrorMessage = [self _stringForCString:mysql_error(mySQLConnection)];
-	NSUInteger theErrorID = mysql_errno(mySQLConnection);
+		// Update the error message, if appropriate, to reflect result store errors or overall success
+		theErrorMessage = [self _stringForCString:mysql_error(mySQLConnection)];
+		theErrorID = mysql_errno(mySQLConnection);
+	}
 
 	// Update the connection's stored insert ID if available
 	if (mySQLConnection->insert_id) {
@@ -350,6 +359,7 @@
 		// after query kills.  This is also handled within the class for internal cancellations, but
 		// as other external classes may also cancel the query.
 		if (![self serverVersionIsGreaterThanOrEqualTo:5 minorVersion:0 releaseVersion:0]) {
+			[self _unlockConnection];
 			[self checkConnection];
 		}
 	}
diff --git a/Frameworks/SPMySQLFramework/Source/SPMySQLConnection.h b/Frameworks/SPMySQLFramework/Source/SPMySQLConnection.h
index 3400ecfd..5f5bb24e 100644
--- a/Frameworks/SPMySQLFramework/Source/SPMySQLConnection.h
+++ b/Frameworks/SPMySQLFramework/Source/SPMySQLConnection.h
@@ -59,7 +59,7 @@
 	SPMySQLConnectionState state;
 	BOOL connectedWithSSL;
 	BOOL userTriggeredDisconnect;
-	BOOL isReconnecting;
+	pthread_t reconnectingThread;
 	uint64_t initialConnectTime;
 	unsigned long mysqlConnectionThreadId;
 
diff --git a/Frameworks/SPMySQLFramework/Source/SPMySQLConnection.m b/Frameworks/SPMySQLFramework/Source/SPMySQLConnection.m
index 4dbed002..1470d1bf 100644
--- a/Frameworks/SPMySQLFramework/Source/SPMySQLConnection.m
+++ b/Frameworks/SPMySQLFramework/Source/SPMySQLConnection.m
@@ -108,7 +108,7 @@ const char *SPMySQLSSLPermissibleCiphers = "DHE-RSA-AES256-SHA:AES256-SHA:DHE-RS
 		mySQLConnection = NULL;
 		state = SPMySQLDisconnected;
 		userTriggeredDisconnect = NO;
-		isReconnecting = NO;
+		reconnectingThread = NULL;
 		mysqlConnectionThreadId = 0;
 		initialConnectTime = 0;
 
@@ -618,11 +618,11 @@ const char *SPMySQLSSLPermissibleCiphers = "DHE-RSA-AES256-SHA:AES256-SHA:DHE-RS
 	// Check whether a reconnection attempt is already being made - if so, wait
 	// and return the status of that reconnection attempt.  This improves threaded
 	// use of the connection by preventing reconnect races.
-	if (isReconnecting) {
+	if (reconnectingThread && !pthread_equal(reconnectingThread, pthread_self())) {
 
 		// Loop in a panel runloop mode until the reconnection has processed; if an iteration
 		// takes less than the requested 0.1s, sleep instead.
-		while (isReconnecting) {
+		while (reconnectingThread) {
 			uint64_t loopIterationStart_t = mach_absolute_time();
 
 			[[NSRunLoop currentRunLoop] runMode:NSModalPanelRunLoopMode beforeDate:[NSDate dateWithTimeIntervalSinceNow:0.1]];
@@ -643,7 +643,7 @@ const char *SPMySQLSSLPermissibleCiphers = "DHE-RSA-AES256-SHA:AES256-SHA:DHE-RS
 		return NO;
 	}
 
-	isReconnecting = YES;
+	reconnectingThread = pthread_self();
 
 	// Store certain details about the connection, so that if the reconnection is successful
 	// they can be restored.  This has to be treated separately from _restoreConnectionDetails
@@ -667,7 +667,8 @@ const char *SPMySQLSSLPermissibleCiphers = "DHE-RSA-AES256-SHA:AES256-SHA:DHE-RS
 	[self _waitForNetworkConnectionWithTimeout:10];
 
 	if ([[NSThread currentThread] isCancelled]) {
-		isReconnecting = NO;
+		[self _unlockConnection];
+		reconnectingThread = NULL;
 		[reconnectionPool release];
 		return NO;
 	}
@@ -783,12 +784,12 @@ const char *SPMySQLSSLPermissibleCiphers = "DHE-RSA-AES256-SHA:AES256-SHA:DHE-RS
 
 			// By default attempt a reconnect
 			default:
-				isReconnecting = NO;
+				reconnectingThread = NULL;
 				reconnectSucceeded = [self _reconnectAllowingRetries:YES];
 		}
 	}
 
-	isReconnecting = NO;
+	reconnectingThread = NULL;
 	[reconnectionPool release];
 	return reconnectSucceeded;
 }
@@ -897,10 +898,13 @@ const char *SPMySQLSSLPermissibleCiphers = "DHE-RSA-AES256-SHA:AES256-SHA:DHE-RS
 	encodingUsesLatin1Transport = NO;
 
 	// Check the interactive timeout - if it's below five minutes, increase it to ten
-	// to imprive timeout/keepalive behaviour
+	// to improve timeout/keepalive behaviour.  Note that wait_timeout also has be
+	// increased; current versions effectively populate the wait timeout from the
+	// interactive_timeout for interactive clients, but don't pick up changes.
 	if ([variables objectForKey:@"interactive_timeout"]) {
 		if ([[variables objectForKey:@"interactive_timeout"] integerValue] < 300) {
 			[self queryString:@"SET interactive_timeout=600"];
+			[self queryString:@"SET wait_timeout=600"];
 		}
 	}