Sets and enforces a connection timeout, and handles connection timeouts appropriately - offering to retry, reconnect, or disconnect. This fixes Issue #93, Issue #69, and Issue #77.

The gory details:
Previously, MCPKit was correctly running mysql_ping to ensure a connection still existed before running a query, and aborted the query if the connection was no longer active.However the code very rarely checked the response of this, so if a query failed subsequent queries would continue to be run and the program would end up checking non-existent results, throwing Cocoa exceptions and generally breaking.  However, mysql_ping would also use the default timeout (30 seconds) for each check - when running the (previous to r333) 14 queries to switch tables, this resulted in a long hang before the program even broke.

To exacerbate the issue, certain situations triggered a bug present in mysql_ping in the old client binaries we're using (http://bugs.mysql.com/bug.php?id=9678), causing mysql_ping to never return despite the presence of a timeout, and so causing an indefinite hang.

This issue has been fixed by:
 - Setting a new 10 second connection timeout for both new connections (Issue #69) and for mysql_pings.  Once preferences have been redesigned we'll probably make this value editable.
 - Enforce the 10 second timeout even if mysql_ping hangs by using interrupts.
 - Wrap mysql_ping in a new method to do the above and also catch re-established connections without reporting false failures.
 - When a connection has failed, prompt the user to Retry, Reconnect, or Disconnect.  Reconnect uses the original details for the old connection to establish a new connection, also attempting to preserve the current encoding.
 - Do not return control to the main loop until a connection has been reestablished (or disconnected) - this ensures the program is never in a broken state without having to rewrite all query usage.
Much of the above patches the MCPKit connection methods as necessary.

1 files changed, 312 insertions, 18 deletions

diff --git a/Source/CMMCPConnection.m b/Source/CMMCPConnection.m
index 8d4e2673..20f5ba11 100644
--- a/Source/CMMCPConnection.m
+++ b/Source/CMMCPConnection.m
@@ -23,10 +23,191 @@
 //  Or mail to <lorenz@textor.ch>
 
 #import "CMMCPConnection.h"
+#include <unistd.h>
+#include <setjmp.h>
 
+static jmp_buf pingTimeoutJumpLocation;
+static void forcePingTimeout(int signalNumber);
 
 @implementation CMMCPConnection
 
+
+/*
+ * Override the normal init methods, extending them to also init additional details.
+ */
+- (id) init
+{
+	[self initSPExtensions];
+	self = [super init];
+	return self;
+}
+- (id) initToHost:(NSString *) host withLogin:(NSString *) login password:(NSString *) pass usingPort:(int) port
+{
+	[self initSPExtensions];
+	self = [super initToHost:host withLogin:login password:pass usingPort:port];
+	return self;
+}
+- (id) initToSocket:(NSString *) socket withLogin:(NSString *) login password:(NSString *) pass
+{
+	[self initSPExtensions];
+	self = [super initToSocket:socket withLogin:login password:pass];
+	return self;
+}
+
+
+/*
+ * Instantiate extra variables and load the connection error dialog for potential use.
+ */
+- (void) initSPExtensions
+{
+	parentWindow = nil;
+	connectionLogin = nil;
+	connectionPassword = nil;
+	connectionHost = nil;
+	connectionPort = 0;
+	connectionSocket = nil;
+	[NSBundle loadNibNamed:@"ConnectionErrorDialog" owner:self];
+}
+
+
+/*
+ * Override the normal connection method, extending it to also store details of the
+ * current connection to allow reconnection as necessary.  This also sets the connection timeout
+ * - used for pings, not for long-running commands.
+ */
+- (BOOL) connectWithLogin:(NSString *) login password:(NSString *) pass host:(NSString *) host port:(int) port socket:(NSString *) socket
+{
+	if (connectionLogin) [connectionLogin release];
+	if (login) connectionLogin = [[NSString alloc] initWithString:login];
+	if (connectionPassword) [connectionPassword release];
+	if (pass) connectionPassword = [[NSString alloc] initWithString:pass];
+	if (connectionHost) [connectionHost release];
+	if (host) connectionHost = [[NSString alloc] initWithString:host];
+	connectionPort = port;
+	if (connectionSocket) [connectionSocket release];
+	if (socket) connectionSocket = [[NSString alloc] initWithString:socket];
+
+	if (mConnection != NULL) {
+		unsigned int connectionTimeout = SP_CONNECTION_TIMEOUT;
+		mysql_options(mConnection, MYSQL_OPT_CONNECT_TIMEOUT, (const void *)&connectionTimeout);
+	}
+
+	return [super connectWithLogin:login password:pass host:host port:port socket:socket];
+}
+
+
+/*
+ * Override the stored disconnection method to ensure that disconnecting clears stored details.
+ */
+- (void) disconnect
+{
+	[super disconnect];
+
+	if (connectionLogin) [connectionLogin release];
+	connectionLogin = nil;
+	if (connectionPassword) [connectionPassword release];
+	connectionPassword = nil;
+	if (connectionHost) [connectionHost release];
+	connectionHost = nil;
+	connectionPort = 0;
+	if (connectionSocket) [connectionSocket release];
+	connectionSocket = nil;
+}
+
+
+/*
+ * Reconnect to the currently "active" - but possibly disconnected - connection, using the
+ * stored details.
+ * Error checks extensively - if this method fails, it will ask how to proceed and loop depending
+ * on the status, not returning control until either a connection has been established or
+ * the connection and document have been closed.
+ */
+- (BOOL) reconnect
+{
+	NSString *currentEncoding = nil;
+	NSString *currentDatabase = nil;
+
+	// Store the current database and encoding so they can be re-set if reconnection was successful
+	if (delegate && [delegate valueForKey:@"selectedDatabase"]) {
+		currentDatabase = [NSString stringWithString:[delegate valueForKey:@"selectedDatabase"]];
+	}
+	if (delegate && [delegate valueForKey:@"_encoding"]) {
+		currentEncoding = [NSString stringWithString:[delegate valueForKey:@"_encoding"]];
+	}
+
+	// Close the connection if it exists.
+	if (mConnected) {
+		mysql_close(mConnection);
+		mConnection = NULL;
+	}
+	mConnected = NO;
+
+	// Attempt to reinitialise the connection - if this fails, it will still be set to NULL.
+	if (mConnection == NULL) {
+		mConnection = mysql_init(NULL);
+	}
+
+	if (mConnection != NULL) {
+
+		// Set a connection timeout for the new connection
+		unsigned int connectionTimeout = SP_CONNECTION_TIMEOUT;
+		mysql_options(mConnection, MYSQL_OPT_CONNECT_TIMEOUT, (const void *)&connectionTimeout);
+
+		// Attempt to reestablish the connection - using own method so everything gets set up as standard.
+		// Will store the supplied details again, which isn't a problem.
+		[self connectWithLogin:connectionLogin password:connectionPassword host:connectionHost port:connectionPort socket:connectionSocket];
+	}
+
+	// If the connection was successfully established, reselect the old database and encoding if appropriate.
+	if (mConnected) {
+		if (currentDatabase) {
+			[self selectDB:currentDatabase];
+		}
+		if (currentEncoding) {
+			[self queryString:[NSString stringWithFormat:@"SET NAMES '%@'", currentEncoding]];
+			[self setEncoding:[CMMCPConnection encodingForMySQLEncoding:[currentEncoding UTF8String]]];
+		}
+	} else if (parentWindow) {
+
+		// If the connection was not successfully established, ask how to proceed.
+		[NSApp beginSheet:connectionErrorDialog modalForWindow:parentWindow modalDelegate:self didEndSelector:nil contextInfo:nil];
+		int connectionErrorCode = [NSApp runModalForWindow:connectionErrorDialog];
+		[NSApp endSheet:connectionErrorDialog];
+		[connectionErrorDialog orderOut:nil];
+
+		switch (connectionErrorCode) {
+
+				// Should disconnect
+				case 2:
+					[parentWindow close];
+					return NO;
+
+				// Should retry
+				default:
+					return [self reconnect];
+		}
+	}
+
+	return mConnected;
+}
+
+
+/*
+ * Set the parent window of the connection for use with dialogs.
+ */
+- (void)setParentWindow:(NSWindow *)theWindow {
+	parentWindow = theWindow;
+}
+
+
+/*
+ * Ends and existing modal session
+ */
+- (IBAction) closeSheet:(id)sender
+{
+	[NSApp stopModalWithCode:[sender tag]];
+}
+
 /*
 Gets a proper NSStringEncoding according to the given MySQL charset.
 
@@ -92,7 +273,7 @@ WARNING : incomplete implementation. Please, send your fixes.
 	if (!strncmp(mysqlEncoding, "sjis", 4)) {
 		return  NSShiftJISStringEncoding;
 	}
-	
+
 	// default to iso latin 1, even if it is not exact (throw an exception?)
 	NSLog(@"warning: unknown encoding %s! falling back to latin1.", mysqlEncoding);
 	return NSISOLatin1StringEncoding;
@@ -101,7 +282,10 @@ WARNING : incomplete implementation. Please, send your fixes.
 
 
 /*
- modified version of queryString to be used in sequel-pro
+ * Modified version of queryString to be used in Sequel Pro.
+ * Error checks extensively - if this method fails, it will ask how to proceed and loop depending
+ * on the status, not returning control until either the query has been executed and the result can
+ * be returned or the connection and document have been closed.
  */
 - (CMMCPResult *)queryString:(NSString *) query
 {
@@ -109,30 +293,29 @@ WARNING : incomplete implementation. Please, send your fixes.
 	const char	*theCQuery = [self cStringFromString:query];
 	int			theQueryCode;
 
-	// check connection
-	if (![self checkConnection]) {
-		NSLog(@"Connection was gone, but should be reestablished now!");
-	}
+	// If no connection is present, return nil.
+	if (!mConnected) return nil;
+
+	// Check the connection.  This triggers reconnects as necessary, and should only return false if a disconnection
+	// has been requested - in which case return nil
+	if (![self checkConnection]) return nil;
 
-	// inform the delegate about the query
+	// Inform the delegate about the query
 	if (delegate && [delegate respondsToSelector:@selector(willQueryString:)]) {
 		[delegate willQueryString:query];
 	}
 
 	if (0 == (theQueryCode = mysql_query(mConnection, theCQuery))) {
 		if (mysql_field_count(mConnection) != 0) {
-			// use CMMCPResult instad of MCPResult
+
+			// Use CMMCPResult instad of MCPResult
 			theResult = [[CMMCPResult alloc] initWithMySQLPtr:mConnection encoding:mEncoding timeZone:mTimeZone];
 		} else {
 			return nil;
 		}
 	} else {
-//		NSLog (@"Problem in queryString error code is : %d, query is : %s -in ObjC : %@-\n", theQueryCode, theCQuery, query);
-//		NSLog(@"Error message is : %@\n", [self getLastErrorMessage]);
-//		theResult = [theResult init]; // Old version...
-//		theResult = nil;
-		
-		// inform the delegate about errors
+
+		// Inform the delegate about errors
 		if (delegate && [delegate respondsToSelector:@selector(queryGaveError:)]) {
 			[delegate queryGaveError:[self getLastErrorMessage]];
 		}
@@ -142,6 +325,50 @@ WARNING : incomplete implementation. Please, send your fixes.
 	return [theResult autorelease];
 }
 
+static void sigalarm(int segnum) {
+	NSLog(@"BOOOOYAAAAA");
+}
+/*
+ * Checks whether the connection to the server is still active.  If not, prompts for what approach to take,
+ * offering to retry, reconnect or disconnect the connection.
+ */
+- (BOOL)checkConnection
+{
+	if (!mConnected) return NO;
+
+	BOOL connectionVerified = FALSE;
+
+	// Check whether the connection is still operational via a wrapped version of MySQL ping.
+	connectionVerified = [self pingConnection];
+
+	// If the connection doesn't appear to be responding, show a dialog asking how to proceed
+	if (!connectionVerified) {
+		[NSApp beginSheet:connectionErrorDialog modalForWindow:parentWindow modalDelegate:self didEndSelector:nil contextInfo:nil];
+		int responseCode = [NSApp runModalForWindow:connectionErrorDialog];
+		[NSApp endSheet:connectionErrorDialog];
+		[connectionErrorDialog orderOut:nil];
+
+		switch (responseCode) {
+
+			// "Reconnect" has been selected.  Request a reconnect, and retry.
+			case 1:
+				[self reconnect];
+				return [self checkConnection];
+
+			// "Disconnect" has been selected.  Close the parent window, which will handle disconnections, and return false.
+			case 2:
+				[parentWindow close];
+				return FALSE;
+
+			// "Retry" has been selected - return a recursive call.
+			default:
+				return [self checkConnection];
+		}
+	}
+
+	return connectionVerified;
+}
+
 - (void)setDelegate:(id)object
 {
 	delegate = object;
@@ -156,7 +383,7 @@ WARNING : incomplete implementation. Please, send your fixes.
 		NSArray		*theRow;
 		id			theTZName;
 		NSTimeZone	*theTZ;
-		
+
 		[theSessionTZ dataSeek:1ULL];
 		theRow = [theSessionTZ fetchRowAsArray];
 		theTZName = [theRow objectAtIndex:1];
@@ -176,7 +403,7 @@ WARNING : incomplete implementation. Please, send your fixes.
 				theTZName = [self stringWithText:theTZName];
 			}
 		}
-		
+
 		if (theTZName) { // Old versions of the server does not support there own time zone ?
 			theTZ = [NSTimeZone timeZoneWithName:theTZName];
 		} else {
@@ -195,7 +422,7 @@ WARNING : incomplete implementation. Please, send your fixes.
 				NSLog(@"The time zone is not defined on the server, set it to the default one : %@", theTZ);
 			}
 		}
-		
+
 		if (theTZ != mTimeZone) {
 			[mTimeZone release];
 			mTimeZone = [theTZ retain];
@@ -204,4 +431,71 @@ WARNING : incomplete implementation. Please, send your fixes.
 	return mTimeZone;
 }
 
-@end
+
+/*
+ * The current versions of MCPKit (and up to and including 3.0.1) use MySQL 4.1.12; this has an issue with
+ * mysql_ping where a connection which is terminated will cause mysql_ping never to respond, even when
+ * connection timeouts are set.  Full details of this issue are available at http://bugs.mysql.com/bug.php?id=9678 ;
+ * this bug was fixed in 4.1.22 and later versions.
+ * This issue can be replicated by connecting to a remote host, and then configuring a firewall on that host
+ * to drop all packets on the connected port - mysql_ping and so Sequel Pro will hang.
+ * Until the client libraries are updated, this provides a drop-in wrapper for mysql_ping, which calls mysql_ping
+ * while running a SIGALRM to enforce the specified connection time.  This is low-level but effective.
+ * Unlike mysql_ping, this function returns FALSE on failure and TRUE on success.
+ */
+- (BOOL) pingConnection
+{
+	struct sigaction timeoutAction;
+	NSDate *startDate = [NSDate date];
+	BOOL pingSuccess = FALSE;
+	
+	// Construct the SIGALRM to fire after the connection timeout if it isn't cleared, calling the forcePingTimeout function.
+	timeoutAction.sa_handler = forcePingTimeout;
+	sigemptyset(&timeoutAction.sa_mask);
+	timeoutAction.sa_flags = 0;
+	sigaction(SIGALRM, &timeoutAction, NULL);
+	alarm(SP_CONNECTION_TIMEOUT+1);
+
+	// Set up a "restore point", returning 0; if longjmp is used later with this reference, execution
+	// jumps back to this point and returns a nonzero value, so this function evaluates to false when initially
+	// set and true if it's called again.
+	if (setjmp(pingTimeoutJumpLocation)) {
+
+		// The connection timed out - we want to return false.
+		pingSuccess = FALSE;
+	
+	// On direct execution:
+	} else {
+
+		// Run mysql_ping, which returns 0 on success, and otherwise an error.
+		pingSuccess = (BOOL)(! mysql_ping(mConnection));
+
+		// If the ping failed within a second, try another one; this is because a terminated-but-then
+		// restored connection is at times restored or functional after a ping, but the ping still returns
+		// an error.  This additional check ensures the returned status is correct with minimal other effect.
+		if (!pingSuccess && ([[NSDate date] timeIntervalSinceDate:startDate] < 1)) {
+			pingSuccess = (BOOL)(! mysql_ping(mConnection));
+		}
+	}
+
+	// Reset and clear the SIGALRM used to check connection timeouts.
+	alarm(0);
+	timeoutAction.sa_handler = SIG_IGN;
+	sigemptyset(&timeoutAction.sa_mask);
+	timeoutAction.sa_flags = 0;
+	sigaction(SIGALRM, &timeoutAction, NULL);
+	
+	
+	return pingSuccess;
+}
+
+/*
+ * This function is paired with pingConnection, and provides a method of enforcing the connection
+ * timeout when mysql_ping does not respect the specified limits.
+ */
+static void forcePingTimeout(int signalNumber)
+{
+	longjmp(pingTimeoutJumpLocation, 1);
+}
+
+@end
\ No newline at end of file