From e7c321f9af247b84eb566c35e6763324cf36b49b Mon Sep 17 00:00:00 2001 From: rowanbeentje Date: Mon, 15 Jun 2009 22:51:46 +0000 Subject: Rework queryString:withEncoding:, and ensure memory is correctly released where used in our overwrites: - Check allocated memory and ensure it is released in our code - Consolidate code controlling both an initial query and any retries following connection failures - Perform additional checking to ensure a connection error was the cause - Correctly capture any result sets after the max_allowed_packet setting was changed - Check and cache the error strings and numbers for the intended queries, not any helper queries, and return them when requested. --- Source/CMMCPConnection.h | 5 +- Source/CMMCPConnection.m | 121 ++++++++++++++++++++++++++++++++--------------- 2 files changed, 88 insertions(+), 38 deletions(-) (limited to 'Source') diff --git a/Source/CMMCPConnection.h b/Source/CMMCPConnection.h index f9b8b022..da029e9f 100644 --- a/Source/CMMCPConnection.h +++ b/Source/CMMCPConnection.h @@ -52,12 +52,15 @@ NSString *connectionHost; int connectionPort; NSString *connectionSocket; - float lastQueryExecutionTime; int connectionTimeout; int currentSSHTunnelState; BOOL useKeepAlive; float keepAliveInterval; + float lastQueryExecutionTime; + NSString *lastQueryErrorMessage; + unsigned int lastQueryErrorId; + BOOL isMaxAllowedPacketEditable; NSString *serverVersionString; diff --git a/Source/CMMCPConnection.m b/Source/CMMCPConnection.m index 8b256cc4..5ace2155 100644 --- a/Source/CMMCPConnection.m +++ b/Source/CMMCPConnection.m @@ -115,6 +115,9 @@ static void forcePingTimeout(int signalNumber); - (void) initSPExtensions { parentWindow = nil; + connectionHost = nil; + connectionLogin = nil; + connectionSocket = nil; connectionPassword = nil; connectionKeychainName = nil; connectionKeychainAccount = nil; @@ -127,6 +130,8 @@ static void forcePingTimeout(int signalNumber); if (!keepAliveInterval) keepAliveInterval = 0; lastKeepAliveSuccess = nil; lastQueryExecutionTime = 0; + lastQueryErrorId = 0; + lastQueryErrorMessage = nil; if (![NSBundle loadNibNamed:@"ConnectionErrorDialog" owner:self]) { NSLog(@"Connection error dialog could not be loaded; connection failure handling will not function correctly."); } @@ -657,12 +662,13 @@ static void forcePingTimeout(int signalNumber); { CMMCPResult *theResult = nil; NSDate *queryStartDate; - NSString *queryErrorMessage; const char *theCQuery; int queryResultCode; + int queryErrorId = 0; int currentMaxAllowedPacket = -1; - unsigned int queryErrorID = 0; + BOOL isQueryRetry = NO; unsigned long threadid = mConnection->thread_id; + NSString *queryErrorMessage = nil; // If no connection is present, return nil. if (!mConnected) return nil; @@ -677,61 +683,78 @@ static void forcePingTimeout(int signalNumber); // Derive the query string in the correct encoding theCQuery = [self cStringFromString:query usingEncoding:encoding]; - // Run the query, timing the execution time on the server. Note that this will include network lag. - queryStartDate = [NSDate date]; - queryResultCode = mysql_query(mConnection, theCQuery); - lastQueryExecutionTime = [[NSDate date] timeIntervalSinceDate:queryStartDate]; - if (0 != queryResultCode) queryErrorMessage = [self getLastErrorMessage]; - - // If there was an error, check whether it was a connection-related error - if (0 != queryResultCode && [CMMCPConnection isErrorNumberConnectionError:[self getLastErrorID]]) { - - queryErrorID = [self getLastErrorID]; + // In a loop to allow one reattempt, perform the query. + while (1) { + + // If this query has failed once already, check the connection + if (isQueryRetry) { + if (![self checkConnection]) return nil; - // Check the connection and see if it can be restored. 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; - threadid = mConnection->thread_id; + // If the connection thread is still the same as at the start, the + // connection wasn't the issue - don't retry. + if (threadid == mConnection->thread_id) break; + + // Try to increase the max_allowed_packet after error 2006 if the user + // has SUPER privileges, reconnecting to use the new settings. + if(isMaxAllowedPacketEditable && queryResultCode == 1 && queryErrorId == 2006) { + currentMaxAllowedPacket = [self getMaxAllowedPacket]; + [self setMaxAllowedPacketTo:strlen(theCQuery)+1024 resetSize:NO]; + [self reconnect]; + } - // Try to increase max_allowed_packet for error 2006 if user - // has SUPER privileges - if(isMaxAllowedPacketEditable && queryResultCode == 1 && queryErrorID == 2006) { - currentMaxAllowedPacket = [self getMaxAllowedPacket]; - [self setMaxAllowedPacketTo:strlen(theCQuery)+1024 resetSize:NO]; - [self reconnect]; + threadid = mConnection->thread_id; } - // The connection has been restored - re-run the query + // Run (or re-run) the query, timing the execution time of the query - note + // that this time will include network lag. queryStartDate = [NSDate date]; queryResultCode = mysql_query(mConnection, theCQuery); lastQueryExecutionTime = [[NSDate date] timeIntervalSinceDate:queryStartDate]; - if (0 != queryResultCode) { - queryErrorMessage = [self getLastErrorMessage]; - queryErrorID = [self getLastErrorID]; - } - } - // Store the query result if appropriate - if (0 == queryResultCode && mysql_field_count(mConnection) != 0) { - theResult = [[CMMCPResult alloc] initWithMySQLPtr:mConnection encoding:mEncoding timeZone:mTimeZone]; + // On success, capture the results + if (0 == queryResultCode) { + if (mysql_field_count(mConnection) != 0) + theResult = [[CMMCPResult alloc] initWithMySQLPtr:mConnection encoding:mEncoding timeZone:mTimeZone]; + queryErrorMessage = [[NSString alloc] initWithString:@""]; + queryErrorId = 0; + + // On failure, set the error messages and IDs + } else { + queryErrorMessage = [[NSString alloc] initWithString:[self stringWithCString:mysql_error(mConnection)]]; + queryErrorId = mysql_errno(mConnection); + + // If the error was a connection error, retry once + if (!isQueryRetry && [CMMCPConnection isErrorNumberConnectionError:queryErrorId]) { + isQueryRetry = YES; + continue; + } + } + + break; } - // If the mysql thread id has changed (this seems to happen with large queries - // whatever the packet size limit), ensure connection details are still correct + // If the mysql thread id has changed as a result of a connection error, + // ensure connection details are still correct if (threadid != mConnection->thread_id) [self restoreEncodingDetails]; // If max_allowed_packet was changed, reset it to default if(currentMaxAllowedPacket > -1) [self setMaxAllowedPacketTo:currentMaxAllowedPacket resetSize:YES]; + // Update error strings and IDs + if (lastQueryErrorMessage) [lastQueryErrorMessage release], lastQueryErrorMessage = nil; + lastQueryErrorId = queryErrorId; + lastQueryErrorMessage = [[NSString alloc] initWithString:queryErrorMessage?queryErrorMessage:@""]; + if (queryErrorMessage) [queryErrorMessage release]; + // If an error occurred, inform the delegate if (0 != queryResultCode && delegate && [delegate respondsToSelector:@selector(queryGaveError:)]) { - if (queryResultCode == 1 && queryErrorID == 2006) + if (queryResultCode == 1 && lastQueryErrorId == 2006) // Very likely that query length > max_allowed_packet - [delegate queryGaveError:[NSString stringWithFormat:@"%@ (Please check if query size < max_allowed_packet)", queryErrorMessage]]; + [delegate queryGaveError:[NSString stringWithFormat:@"%@ (Please check if query size < max_allowed_packet)", lastQueryErrorMessage]]; else - [delegate queryGaveError:queryErrorMessage]; + [delegate queryGaveError:lastQueryErrorMessage]; } [self startKeepAliveTimerResettingState:YES]; @@ -779,7 +802,6 @@ static void forcePingTimeout(int signalNumber); // Get the current thread ID for this connection threadid = mConnection->thread_id; -NSLog(@"Connection checking"); // Check whether the connection is still operational via a wrapped version of MySQL ping. connectionVerified = [self pingConnection]; @@ -1044,6 +1066,22 @@ static void forcePingTimeout(int signalNumber) return (const char *)[theData bytes]; } +/* + * Returns a string for the last MySQL error message on the connection. + */ +- (NSString *) getLastErrorMessage +{ + return lastQueryErrorMessage; +} + +/* + * Returns the ErrorID of the last MySQL error on the connection. + */ +- (unsigned int) getLastErrorID +{ + return lastQueryErrorId; +} + /* * Retrieves max_allowed_packet size set as global variable. * It returns -1 if it fails. @@ -1098,6 +1136,15 @@ static void forcePingTimeout(int signalNumber) - (void) dealloc { + if (lastQueryErrorMessage) [lastQueryErrorMessage release]; + if (connectionHost) [connectionHost release]; + if (connectionLogin) [connectionLogin release]; + if (connectionSocket) [connectionSocket release]; + if (connectionPassword) [connectionPassword release]; + if (connectionKeychainName) [connectionKeychainName release]; + if (connectionKeychainAccount) [connectionKeychainAccount release]; + if (lastKeepAliveSuccess) [lastKeepAliveSuccess release]; + [super dealloc]; } -- cgit v1.2.3