Improve Disconnection on connection loss:

 - Set error strings on MCPConnection on user disconnect to allow existing error chcking to catch the state
 - Improve close behaviour from threads
 - Improve window close behaviour and appearance
 - Add new checks for disconnection in one or two crash-prone locations
This addresses Issue #531, one of Issue #532, one of Issue #539, and probable reported crashes on Issue #541.

5 files changed, 51 insertions, 22 deletions

diff --git a/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m b/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m
index cda82647..7a3c9c8c 100644
--- a/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m
+++ b/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m
@@ -341,6 +341,7 @@ static BOOL	sTruncateLongFieldInLogs = YES;
 	mConnected = YES;
 	connectionStartTime = mach_absolute_time();
 	mEncoding = [MCPConnection encodingForMySQLEncoding:mysql_character_set_name(mConnection)];
+	[self setLastErrorMessage:nil];
 	connectionThreadId = mConnection->thread_id;
 	[self timeZone]; // Getting the timezone used by the server.
 	
@@ -536,8 +537,12 @@ static BOOL	sTruncateLongFieldInLogs = YES;
 				
 				return [self checkConnection];
 				
-			// 'Disconnect' has been selected. Close the parent window, which will handle disconnections, and return false.
+			// 'Disconnect' has been selected. The parent window should already have
+			// triggered UI-specific actions, and may have disconnected already; if
+			// not, disconnect, and clean up.
 			case MCPConnectionCheckDisconnect:
+				if (mConnected) [self disconnect];
+				[self setLastErrorMessage:NSLocalizedString(@"User triggered disconnection", @"User triggered disconnection")];
 				return NO;
 				
 			// 'Retry' has been selected - return a recursive call.
diff --git a/Source/SPConnectionDelegate.m b/Source/SPConnectionDelegate.m
index 2742c7a8..ff0cca23 100644
--- a/Source/SPConnectionDelegate.m
+++ b/Source/SPConnectionDelegate.m
@@ -118,11 +118,25 @@
 	
 	// If 'disconnect' was selected, trigger a window close.
 	if (connectionErrorCode == MCPConnectionCheckDisconnect) {
-		[self windowWillClose:nil];
-		[tableWindow performSelector:@selector(close) withObject:nil afterDelay:0.0];
+		[self performSelectorOnMainThread:@selector(closeConnection) withObject:nil waitUntilDone:YES];
 	}
 	
 	return connectionErrorCode;
 }
 
+/**
+ * Close the connection - should be performed on the main thread.
+ * First hides the window to give code a little bit of time to clean
+ * everything up before it's all deallocated as a result of the close.
+ * Also sets alpha to fully transparent so accidental dialogs are hidden!
+ */
+- (void) closeConnection
+{
+		_isConnected = NO;
+		[self windowWillClose:nil];
+		[tableWindow orderOut:self];
+		[tableWindow setAlphaValue:0.0];
+		[tableWindow performSelector:@selector(close) withObject:nil afterDelay:1.0];
+}
+
 @end
diff --git a/Source/TableContent.m b/Source/TableContent.m
index 4c8d9775..f5e1feb4 100644
--- a/Source/TableContent.m
+++ b/Source/TableContent.m
@@ -607,8 +607,10 @@
 	[tableContentView performSelectorOnMainThread:@selector(noteNumberOfRowsChanged) withObject:nil waitUntilDone:YES]; 
 	[self setUsedQuery:queryString];
 	streamingResult = [mySQLConnection streamingQueryString:queryString];
-	[self processResultIntoDataStorage:streamingResult approximateRowCount:rowsToLoad];
-	[streamingResult release];
+	if (streamingResult) {
+		[self processResultIntoDataStorage:streamingResult approximateRowCount:rowsToLoad];
+		[streamingResult release];
+	}
 
 	// If the result is empty, and a late page is selected, reset the page
 	if ([prefs boolForKey:SPLimitResults] && queryStringBeforeLimit && !tableRowsCount && ![mySQLConnection queryCancelled]) {
@@ -616,8 +618,10 @@
 		queryString = [NSMutableString stringWithFormat:@"%@ LIMIT 0,%ld", queryStringBeforeLimit, (long)[prefs integerForKey:SPLimitResultsValue]];
 		[self setUsedQuery:queryString];
 		streamingResult = [mySQLConnection streamingQueryString:queryString];
-		[self processResultIntoDataStorage:streamingResult approximateRowCount:[prefs integerForKey:SPLimitResultsValue]];
-		[streamingResult release];
+		if (streamingResult) {
+			[self processResultIntoDataStorage:streamingResult approximateRowCount:[prefs integerForKey:SPLimitResultsValue]];
+			[streamingResult release];
+		}
 	}
 
 	if ([mySQLConnection queryCancelled] || ![[mySQLConnection getLastErrorMessage] isEqualToString:@""])
diff --git a/Source/TableDocument.m b/Source/TableDocument.m
index d0410044..d8d16e85 100644
--- a/Source/TableDocument.m
+++ b/Source/TableDocument.m
@@ -2236,6 +2236,7 @@
 - (void)closeConnection
 {
 	[mySQLConnection disconnect];
+	_isConnected = NO;
 
 	// Disconnected Growl notification
 	[[SPGrowlController sharedGrowlController] notifyWithTitle:@"Disconnected" 
@@ -3425,7 +3426,7 @@
 - (void)windowWillClose:(NSNotification *)aNotification
 {
 	[mySQLConnection setDelegate:nil];
-	if ([mySQLConnection isConnected]) [self closeConnection];
+	if (_isConnected) [self closeConnection];
 	if ([[[SPQueryController sharedQueryController] window] isVisible]) [self toggleConsole:self];
 	[createTableSyntaxWindow orderOut:nil];
 	[[NSNotificationCenter defaultCenter] removeObserver:self];
@@ -3436,22 +3437,26 @@
  */
 - (BOOL)windowShouldClose:(id)sender
 {
-	if (_isWorkingLevel) {
-		return NO;
-	} else if ( ![tablesListInstance selectionShouldChangeInTableView:nil] ) {
-		return NO;
-	} else {
 
-		if(!_isConnected) return YES;
+	// If no connection is available, always return YES.  Covers initial setup and disconnections.
+	if(!_isConnected) return YES;
 
-		// Auto-save spf file based connection
-		if([self fileURL] && [[[self fileURL] absoluteString] length] && ![self isUntitled]) {
-			BOOL isSaved = [self saveDocumentWithFilePath:nil inBackground:YES onlyPreferences:YES];
-			if(isSaved)
-				[[SPQueryController sharedQueryController] removeRegisteredDocumentWithFileURL:[self fileURL]];
-			return isSaved;
-		}
+	// If tasks are active, return NO to allow tasks to complete
+	if (_isWorkingLevel) return NO;
+
+	// If the table list considers itself to be working, return NO. This catches open alerts, and
+	// edits in progress in various views.
+	if ( ![tablesListInstance selectionShouldChangeInTableView:nil] ) return NO;
+
+	// Auto-save spf file based connection and return whether the save was successful
+	if([self fileURL] && [[[self fileURL] absoluteString] length] && ![self isUntitled]) {
+		BOOL isSaved = [self saveDocumentWithFilePath:nil inBackground:YES onlyPreferences:YES];
+		if(isSaved)
+			[[SPQueryController sharedQueryController] removeRegisteredDocumentWithFileURL:[self fileURL]];
+		return isSaved;
 	}
+
+	// Return YES by default
 	return YES;
 }
 
diff --git a/Source/TablesList.m b/Source/TablesList.m
index e784f862..3130d44f 100644
--- a/Source/TablesList.m
+++ b/Source/TablesList.m
@@ -616,6 +616,7 @@
  */
 - (void)updateSelectionWithTaskString:(NSString *)taskString
 {
+	if (![mySQLConnection isConnected]) return;
 
 	// If there is a multiple or blank selection, clear all views directly.
 	if ( [tablesListView numberOfSelectedRows] != 1 || ![(NSString *)[filteredTables objectAtIndex:[tablesListView selectedRow]] length] ) {
@@ -644,7 +645,7 @@
 }
 
 - (void) updateSelectionTask
-{	
+{
 	NSAutoreleasePool *selectionChangePool = [[NSAutoreleasePool alloc] init];
 	NSString *tableEncoding = nil;