From 9997d982ca4fcbacb04b41c7f7e2aa7dd44db312 Mon Sep 17 00:00:00 2001 From: rowanbeentje Date: Sun, 24 Jan 2010 21:02:18 +0000 Subject: 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. --- Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m | 7 ++++- Source/SPConnectionDelegate.m | 18 ++++++++++-- Source/TableContent.m | 12 +++++--- Source/TableDocument.m | 33 +++++++++++++--------- Source/TablesList.m | 3 +- 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; -- cgit v1.2.3