From 332f6201ce607a6622fadfd3e6426e4571dc035f Mon Sep 17 00:00:00 2001 From: rowanbeentje Date: Tue, 16 Mar 2010 02:04:50 +0000 Subject: - Make a number of changes to attempt to improve disconnection/quit crashes: prevent multiple disconnects, add more checks, cancel current queries, and add a tiny delay to allow mysql cleanup. - Alter MCPStreamingResult to no longer return a retained instance, setting up correct result disposal on autorelease but changing callers to retain as soon as they receive. - Review and change a number of local variables shadowing/shielding other local or global variables. --- Frameworks/MCPKit/MCPFoundationKit/MCPConnection.h | 1 + Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m | 20 +++++++++++++------- .../MCPKit/MCPFoundationKit/MCPStreamingResult.h | 2 ++ .../MCPKit/MCPFoundationKit/MCPStreamingResult.m | 7 ++++++- Source/CMTextView.m | 2 -- Source/CustomQuery.m | 7 ++++--- Source/SPCSVParser.m | 5 ++--- Source/SPTableData.m | 2 +- Source/SPTableRelations.m | 6 +++--- Source/SPUserManager.m | 4 ++-- Source/TableContent.m | 6 +++--- Source/TableDump.m | 10 +++++----- Source/TableSource.m | 4 ++-- Source/TablesList.m | 8 ++++---- 14 files changed, 48 insertions(+), 36 deletions(-) diff --git a/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.h b/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.h index f04fb26e..8e5a60f9 100644 --- a/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.h +++ b/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.h @@ -82,6 +82,7 @@ static inline NSData* NSStringDataUsingLossyEncoding(NSString* self, NSInteger e NSLock *queryLock; /* Anything that performs a mysql_net_read is not thread-safe: mysql queries, pings */ BOOL useKeepAlive; + BOOL isDisconnecting; NSInteger connectionTimeout; CGFloat keepAliveInterval; diff --git a/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m b/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m index f04fe226..607e1b8e 100644 --- a/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m +++ b/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m @@ -106,6 +106,7 @@ static BOOL sTruncateLongFieldInLogs = YES; queryCancelUsedReconnect = NO; serverVersionString = nil; mTimeZone = nil; + isDisconnecting = NO; // Initialize ivar defaults connectionTimeout = 10; @@ -298,7 +299,7 @@ static BOOL sTruncateLongFieldInLogs = YES; if (mConnected && newState == PROXY_STATE_IDLE && currentProxyState == PROXY_STATE_CONNECTED) { currentProxyState = newState; [connectionProxy setConnectionStateChangeSelector:nil delegate:nil]; - [self reconnect]; + if (!isDisconnecting) [self reconnect]; return; } @@ -404,14 +405,22 @@ static BOOL sTruncateLongFieldInLogs = YES; */ - (void)disconnect { + if (isDisconnecting) return; + isDisconnecting = YES; + [self stopKeepAliveTimer]; if (mConnected) { + [self cancelCurrentQuery]; + mConnected = NO; + + // Small pause for cleanup. + usleep(100000); mysql_close(mConnection); mConnection = NULL; } - mConnected = NO; + isDisconnecting = NO; if (connectionProxy) { [connectionProxy performSelectorOnMainThread:@selector(disconnect) withObject:nil waitUntilDone:YES]; @@ -457,6 +466,7 @@ static BOOL sTruncateLongFieldInLogs = YES; } mConnected = NO; + isDisconnecting = NO; // If there is a tunnel, ensure it's disconnected and attempt to reconnect it in blocking fashion if (connectionProxy) { @@ -1290,7 +1300,6 @@ void performThreadedKeepAlive(void *ptr) /** * Takes a query string and returns an MCPStreamingResult representing the result of the query. - * The returned MCPStreamingResult is retained and the client is responsible for releasing it. * If no fields are present in the result, nil will be returned. * Uses safe/fast mode, which may use more memory as results are downloaded. */ @@ -1301,7 +1310,6 @@ void performThreadedKeepAlive(void *ptr) /** * Takes a query string and returns an MCPStreamingResult representing the result of the query. - * The returned MCPStreamingResult is retained and the client is responsible for releasing it. * If no fields are present in the result, nil will be returned. * Can be used in either fast/safe mode, where data is downloaded as fast as possible to avoid * blocking the server, or in full streaming mode for lowest memory usage but potentially blocking @@ -1316,7 +1324,6 @@ void performThreadedKeepAlive(void *ptr) * Error checks connection extensively - if this method fails due to a connection error, 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. - * If using streamingResult, the caller is responsible for releasing the result set. */ - (id)queryString:(NSString *) query usingEncoding:(NSStringEncoding) encoding streamingResult:(NSInteger) streamResultType { @@ -1516,7 +1523,6 @@ void performThreadedKeepAlive(void *ptr) (void)(*startKeepAliveTimerPtr)(self, startKeepAliveTimerSEL, YES); if (!theResult) return nil; - if (streamResultType != MCP_NO_STREAMING) return theResult; return [theResult autorelease]; } @@ -1618,7 +1624,7 @@ void performThreadedKeepAlive(void *ptr) // Reset the connection [self unlockConnection]; - [self reconnect]; + if (!isDisconnecting) [self reconnect]; // Set queryCancelled again to handle requery cleanups, and return. queryCancelled = YES; diff --git a/Frameworks/MCPKit/MCPFoundationKit/MCPStreamingResult.h b/Frameworks/MCPKit/MCPFoundationKit/MCPStreamingResult.h index 2f5ec638..65ee6423 100644 --- a/Frameworks/MCPKit/MCPFoundationKit/MCPStreamingResult.h +++ b/Frameworks/MCPKit/MCPFoundationKit/MCPStreamingResult.h @@ -53,6 +53,8 @@ typedef struct SP_MYSQL_ROWS { unsigned long freedRowCount; pthread_mutex_t dataCreationLock; pthread_mutex_t dataFreeLock; + IMP isConnectedPtr; + SEL isConnectedSEL; } - (id)initWithMySQLPtr:(MYSQL *)mySQLPtr encoding:(NSStringEncoding)theEncoding timeZone:(NSTimeZone *)theTimeZone connection:(MCPConnection *)theConnection; diff --git a/Frameworks/MCPKit/MCPFoundationKit/MCPStreamingResult.m b/Frameworks/MCPKit/MCPFoundationKit/MCPStreamingResult.m index 624f132c..fd7d181b 100644 --- a/Frameworks/MCPKit/MCPFoundationKit/MCPStreamingResult.m +++ b/Frameworks/MCPKit/MCPFoundationKit/MCPStreamingResult.m @@ -95,6 +95,10 @@ mNumOfFields = 0; } + // Obtain SEL references and pointer + isConnectedSEL = @selector(isConnected); + isConnectedPtr = [parentConnection methodForSelector:isConnectedSEL]; + // If the result is opened in download-data-fast safe mode, set up the additional variables // and threads required. if (!fullyStreaming) { @@ -127,6 +131,7 @@ */ - (void) dealloc { + [self cancelResultLoad]; if (!connectionUnlocked) [parentConnection unlockConnection]; if (!fullyStreaming) { @@ -406,7 +411,7 @@ size_t sizeOfDataLengths = (size_t)(sizeof(unsigned long) * mNumOfFields); // Loop through the rows until the end of the data is reached - indicated via a NULL - while (theRow = mysql_fetch_row(mResult)) { + while ( (BOOL)(*isConnectedPtr)(parentConnection, isConnectedSEL) && (theRow = mysql_fetch_row(mResult))) { // Retrieve the lengths of the returned data fieldLengths = mysql_fetch_lengths(mResult); diff --git a/Source/CMTextView.m b/Source/CMTextView.m index 6e55ec6b..50099ede 100644 --- a/Source/CMTextView.m +++ b/Source/CMTextView.m @@ -968,7 +968,6 @@ NSInteger alphabeticSort(id string1, id string2, void *reverse) // Indent the currently selected line if the caret is within a single line if ([self selectedRange].length == 0) { - NSRange currentLineRange; // Extract the current line range based on the text caret currentLineRange = [textViewString lineRangeForRange:[self selectedRange]]; @@ -1021,7 +1020,6 @@ NSInteger alphabeticSort(id string1, id string2, void *reverse) // Undent the currently selected line if the caret is within a single line if ([self selectedRange].length == 0) { - NSRange currentLineRange; // Extract the current line range based on the text caret currentLineRange = [textViewString lineRangeForRange:[self selectedRange]]; diff --git a/Source/CustomQuery.m b/Source/CustomQuery.m index 1427291c..35338463 100644 --- a/Source/CustomQuery.m +++ b/Source/CustomQuery.m @@ -486,7 +486,7 @@ SEL callbackMethod = NULL; NSString *taskButtonString; - NSInteger i, j, totalQueriesRun = 0, totalAffectedRows = 0; + NSInteger i, totalQueriesRun = 0, totalAffectedRows = 0; double executionTime = 0; NSInteger firstErrorOccuredInQuery = -1; BOOL suppressErrorSheet = NO; @@ -548,7 +548,7 @@ [tempQueries addObject:query]; // Run the query, timing execution (note this also includes network and overhead) - streamingResult = [mySQLConnection streamingQueryString:query]; + streamingResult = [[mySQLConnection streamingQueryString:query] retain]; executionTime += [mySQLConnection lastQueryExecutionTime]; totalQueriesRun++; @@ -575,6 +575,8 @@ else if ( [streamingResult numOfRows] ) totalAffectedRows += [streamingResult numOfRows]; + [streamingResult release]; + // Store any error messages if ( ![[mySQLConnection getLastErrorMessage] isEqualToString:@""] || [mySQLConnection queryCancelled]) { @@ -731,7 +733,6 @@ // If no results were returned, redraw the empty table and post notifications before returning. if ( !resultDataCount ) { [customQueryView performSelectorOnMainThread:@selector(reloadData) withObject:nil waitUntilDone:YES]; - if (streamingResult) [streamingResult release]; // Notify any listeners that the query has completed [[NSNotificationCenter defaultCenter] postNotificationName:@"SMySQLQueryHasBeenPerformed" object:tableDocumentInstance]; diff --git a/Source/SPCSVParser.m b/Source/SPCSVParser.m index 58b70942..d9a77a62 100644 --- a/Source/SPCSVParser.m +++ b/Source/SPCSVParser.m @@ -36,7 +36,7 @@ /** * Retrieve the entire two-dimensional array represented by the current string. - * Serves as a concenience method and also an example of how to use getRow:. + * Serves as a convenience method and also an example of how to use getRow:. */ - (NSArray *) array { @@ -54,8 +54,7 @@ } // Return the array - [csvArray autorelease]; - return [NSArray arrayWithArray:csvArray]; + return [csvArray autorelease]; } /** diff --git a/Source/SPTableData.m b/Source/SPTableData.m index f7450e62..34ab2358 100644 --- a/Source/SPTableData.m +++ b/Source/SPTableData.m @@ -614,7 +614,7 @@ [triggers removeAllObjects]; if( [theResult numOfRows] ) { - for(int i=0; i<[theResult numOfRows]; i++){ + for(i=0; i<[theResult numOfRows]; i++){ [triggers addObject:[theResult fetchRowAsDictionary]]; } } diff --git a/Source/SPTableRelations.m b/Source/SPTableRelations.m index 7d8ef9e3..043b5d66 100644 --- a/Source/SPTableRelations.m +++ b/Source/SPTableRelations.m @@ -535,10 +535,10 @@ NSMutableArray *validColumns = [NSMutableArray array]; // Only add columns of the same data type - for (NSDictionary *column in columns) + for (NSDictionary *aColumn in columns) { - if ([[columnInfo objectForKey:@"type"] isEqualToString:[column objectForKey:@"type"]]) { - [validColumns addObject:[column objectForKey:@"name"]]; + if ([[columnInfo objectForKey:@"type"] isEqualToString:[aColumn objectForKey:@"type"]]) { + [validColumns addObject:[aColumn objectForKey:@"name"]]; } } diff --git a/Source/SPUserManager.m b/Source/SPUserManager.m index 9ce7e5d4..84053fbb 100644 --- a/Source/SPUserManager.m +++ b/Source/SPUserManager.m @@ -1267,8 +1267,8 @@ // Remove items from available so they can't be added twice. NSPredicate *predicate = [NSPredicate predicateWithFormat:@"displayName like[cd] %@", displayName]; - NSArray *results = [[availableController arrangedObjects] filteredArrayUsingPredicate:predicate]; - for (NSDictionary *dict in results) + NSArray *previousObjects = [[availableController arrangedObjects] filteredArrayUsingPredicate:predicate]; + for (NSDictionary *dict in previousObjects) { [availableController removeObject:dict]; } diff --git a/Source/TableContent.m b/Source/TableContent.m index 4c9336d8..b0e5e8fe 100644 --- a/Source/TableContent.m +++ b/Source/TableContent.m @@ -609,7 +609,7 @@ // Perform and process the query [tableContentView performSelectorOnMainThread:@selector(noteNumberOfRowsChanged) withObject:nil waitUntilDone:YES]; [self setUsedQuery:queryString]; - streamingResult = [mySQLConnection streamingQueryString:queryString]; + streamingResult = [[mySQLConnection streamingQueryString:queryString] retain]; // Ensure the number of columns are unchanged; if the column count has changed, abort the load // and queue a full table reload. @@ -624,15 +624,15 @@ // Process the result into the data store if (!fullTableReloadRequired && streamingResult) { [self processResultIntoDataStorage:streamingResult approximateRowCount:rowsToLoad]; - [streamingResult release]; } + if (streamingResult) [streamingResult release]; // If the result is empty, and a late page is selected, reset the page if (!fullTableReloadRequired && [prefs boolForKey:SPLimitResults] && queryStringBeforeLimit && !tableRowsCount && ![mySQLConnection queryCancelled]) { contentPage = 1; queryString = [NSMutableString stringWithFormat:@"%@ LIMIT 0,%ld", queryStringBeforeLimit, (long)[prefs integerForKey:SPLimitResultsValue]]; [self setUsedQuery:queryString]; - streamingResult = [mySQLConnection streamingQueryString:queryString]; + streamingResult = [[mySQLConnection streamingQueryString:queryString] retain]; if (streamingResult) { [self processResultIntoDataStorage:streamingResult approximateRowCount:[prefs integerForKey:SPLimitResultsValue]]; [streamingResult release]; diff --git a/Source/TableDump.m b/Source/TableDump.m index 90b753a6..d50d2b85 100644 --- a/Source/TableDump.m +++ b/Source/TableDump.m @@ -1311,7 +1311,7 @@ NSMutableString *setString = [NSMutableString stringWithString:@""]; NSMutableString *whereString = [NSMutableString stringWithString:@"WHERE "]; - NSInteger i, j; + NSInteger i; NSInteger mapColumn; id cellData; NSInteger mappingArrayCount = [fieldMappingArray count]; @@ -1595,7 +1595,7 @@ rowCount = [[[[mySQLConnection queryString:[NSString stringWithFormat:@"SELECT COUNT(1) FROM %@", [tableName backtickQuotedString]]] fetchRowAsArray] objectAtIndex:0] integerValue]; // Set up a result set in streaming mode - streamingResult = [mySQLConnection streamingQueryString:[NSString stringWithFormat:@"SELECT * FROM %@", [tableName backtickQuotedString]] useLowMemoryBlockingStreaming:([sqlFullStreamingSwitch state] == NSOnState)]; + streamingResult = [[mySQLConnection streamingQueryString:[NSString stringWithFormat:@"SELECT * FROM %@", [tableName backtickQuotedString]] useLowMemoryBlockingStreaming:([sqlFullStreamingSwitch state] == NSOnState)] retain]; fieldNames = [streamingResult fetchFieldNames]; // Update the progress text and set the progress bar back to determinate @@ -1732,7 +1732,7 @@ [metaString setString:@"\n"]; [metaString appendString:@"DELIMITER ;;\n"]; - for (int t=0; t<[queryResult numOfRows]; t++) { + for (int s=0; s<[queryResult numOfRows]; s++) { NSDictionary *triggers = [[NSDictionary alloc] initWithDictionary:[queryResult fetchRowAsDictionary]]; //Definer is user@host but we need to escape it to `user`@`host` @@ -1809,7 +1809,7 @@ [metaString appendString:@"DELIMITER ;;\n"]; // Loop through the definitions, exporting if enabled - for (int t=0; t<[queryResult numOfRows]; t++) { + for (int s=0; s<[queryResult numOfRows]; s++) { NSDictionary *proceduresList = [[NSDictionary alloc] initWithDictionary:[queryResult fetchRowAsDictionary]]; NSString *procedureName = [NSString stringWithFormat:@"%@", [proceduresList objectForKey:@"Name"]]; @@ -2641,7 +2641,7 @@ // Perform a COUNT for progress purposes and make a streaming request for the data streamingResultCount = [[[[mySQLConnection queryString:[NSString stringWithFormat:@"SELECT COUNT(1) FROM %@", [tableName backtickQuotedString]]] fetchRowAsArray] objectAtIndex:0] integerValue]; - streamingResult = [mySQLConnection streamingQueryString:[NSString stringWithFormat:@"SELECT * FROM %@", [tableName backtickQuotedString]] useLowMemoryBlockingStreaming:useLowMemoryBlockingStreaming]; + streamingResult = [[mySQLConnection streamingQueryString:[NSString stringWithFormat:@"SELECT * FROM %@", [tableName backtickQuotedString]] useLowMemoryBlockingStreaming:useLowMemoryBlockingStreaming] retain]; // Note any errors during initial query if ( ![[mySQLConnection getLastErrorMessage] isEqualToString:@""] ) { diff --git a/Source/TableSource.m b/Source/TableSource.m index c5e2a4b8..da5577de 100644 --- a/Source/TableSource.m +++ b/Source/TableSource.m @@ -635,8 +635,8 @@ fetches the result as an array with a dictionary for each row in it //use NULL string from preferences instead of the NSNull oject returned by the framework keys = [tempRow allKeys]; - for (NSInteger i = 0; i < [keys count] ; i++) { - key = NSArrayObjectAtIndex(keys, i); + for (NSInteger j = 0; j < [keys count] ; j++) { + key = NSArrayObjectAtIndex(keys, j); if ( [[tempRow objectForKey:key] isMemberOfClass:nullClass] ) [tempRow setObject:prefsNullValue forKey:key]; } diff --git a/Source/TablesList.m b/Source/TablesList.m index 1b83cfed..74f4a390 100644 --- a/Source/TablesList.m +++ b/Source/TablesList.m @@ -2162,13 +2162,13 @@ // Insert the new item into the tables list and select it. NSInteger addItemAtIndex = NSNotFound; for (NSInteger i = 0; i < [tables count]; i++) { - NSInteger tableType = [[tableTypes objectAtIndex:i] integerValue]; - if (tableType == SP_TABLETYPE_NONE) continue; - if ((tableType == SP_TABLETYPE_VIEW || tableType == SP_TABLETYPE_TABLE) + NSInteger theTableType = [[tableTypes objectAtIndex:i] integerValue]; + if (theTableType == SP_TABLETYPE_NONE) continue; + if ((theTableType == SP_TABLETYPE_VIEW || theTableType == SP_TABLETYPE_TABLE) && (tblType == SP_TABLETYPE_PROC || tblType == SP_TABLETYPE_FUNC)) { continue; } - if ((tableType == SP_TABLETYPE_PROC || tableType == SP_TABLETYPE_FUNC) + if ((theTableType == SP_TABLETYPE_PROC || theTableType == SP_TABLETYPE_FUNC) && (tblType == SP_TABLETYPE_VIEW || tblType == SP_TABLETYPE_TABLE)) { addItemAtIndex = i - 1; break; -- cgit v1.2.3