From 6fb53eb4a62184f5ed0d8bdf42ff0973dd552ff9 Mon Sep 17 00:00:00 2001 From: rowanbeentje Date: Fri, 26 Mar 2010 01:36:54 +0000 Subject: - Simplify connection keepalives, moving to a single repeating timer on the main thread. This cleans up calls, results in small speed improvements when making lots of queries (no more stopping/starting of keepalives), and ensures only the main thread handles the initial keepalives. - Move connection unlocking to a blocking call to fix http://log.sequelpro.com/view/73 - Fix memory leaks caused by non-detached pthreads --- Frameworks/MCPKit/MCPFoundationKit/MCPConnection.h | 7 +- Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m | 119 +++++++-------------- 2 files changed, 37 insertions(+), 89 deletions(-) (limited to 'Frameworks') diff --git a/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.h b/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.h index af38e99c..c6d81bd6 100644 --- a/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.h +++ b/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.h @@ -112,6 +112,7 @@ static inline NSData* NSStringDataUsingLossyEncoding(NSString* self, NSInteger e NSArray *allKeysofDbStructure; NSTimer *keepAliveTimer; + double lastKeepAliveTime; pthread_t keepAliveThread; pthread_t pingThread; uint64_t connectionStartTime; @@ -127,15 +128,11 @@ static inline NSData* NSStringDataUsingLossyEncoding(NSString* self, NSInteger e // Pointers IMP cStringPtr; IMP willQueryStringPtr; - IMP stopKeepAliveTimerPtr; - IMP startKeepAliveTimerPtr; IMP timeConnectedPtr; // Selectors SEL cStringSEL; SEL willQueryStringSEL; - SEL stopKeepAliveTimerSEL; - SEL startKeepAliveTimerSEL; SEL timeConnectedSEL; } @@ -173,8 +170,6 @@ static inline NSData* NSStringDataUsingLossyEncoding(NSString* self, NSInteger e - (BOOL)checkConnection; - (BOOL)pingConnection; void pingConnectionTask(void *ptr); -- (void)startKeepAliveTimer; -- (void)stopKeepAliveTimer; - (void)keepAlive:(NSTimer *)theTimer; - (void)threadedKeepAlive; void performThreadedKeepAlive(void *ptr); diff --git a/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m b/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m index 84fb6a5d..6f516157 100644 --- a/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m +++ b/Frameworks/MCPKit/MCPFoundationKit/MCPConnection.m @@ -95,8 +95,9 @@ static BOOL sTruncateLongFieldInLogs = YES; connectionLogin = nil; connectionSocket = nil; connectionPassword = nil; - keepAliveTimer = nil; + keepAliveTimer = [[NSTimer scheduledTimerWithTimeInterval:10 target:self selector:@selector(keepAlive:) userInfo:nil repeats:YES] retain]; keepAliveThread = NULL; + lastKeepAliveTime = 0; pingThread = NULL; connectionProxy = nil; connectionStartTime = -1; @@ -136,14 +137,10 @@ static BOOL sTruncateLongFieldInLogs = YES; // Obtain SEL references willQueryStringSEL = @selector(willQueryString:connection:); - stopKeepAliveTimerSEL = @selector(stopKeepAliveTimer); - startKeepAliveTimerSEL = @selector(startKeepAliveTimer); cStringSEL = @selector(cStringFromString:); // Obtain pointers cStringPtr = [self methodForSelector:cStringSEL]; - stopKeepAliveTimerPtr = [self methodForSelector:stopKeepAliveTimerSEL]; - startKeepAliveTimerPtr = [self methodForSelector:startKeepAliveTimerSEL]; } return self; @@ -395,8 +392,6 @@ static BOOL sTruncateLongFieldInLogs = YES; return mConnected = NO; } - // Start the keepalive timer - [self startKeepAliveTimer]; return mConnected; } @@ -409,8 +404,6 @@ static BOOL sTruncateLongFieldInLogs = YES; if (isDisconnecting) return; isDisconnecting = YES; - [self stopKeepAliveTimer]; - if (mConnected) { [self cancelCurrentQuery]; mConnected = NO; @@ -461,7 +454,6 @@ static BOOL sTruncateLongFieldInLogs = YES; } // Close the connection if it exists. - [self stopKeepAliveTimer]; if (mConnected) { mysql_close(mConnection); mConnection = NULL; @@ -626,6 +618,9 @@ static BOOL sTruncateLongFieldInLogs = YES; pingActive = YES; // Create a pthread for the ping, so we can force it to end after the connection timeout + pthread_attr_t attr; + pthread_attr_init(&attr); + pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); pthread_create(&pingThread, NULL, (void *)&pingConnectionTask, (void *)mConnection); // Loop tightly until the ping responds, or the elapsed time exceeds the connection timeout @@ -641,6 +636,7 @@ static BOOL sTruncateLongFieldInLogs = YES; pthread_cancel(pingThread); lastPingSuccess = FALSE; } + pthread_attr_destroy(&attr); [queryLock unlock]; @@ -648,7 +644,7 @@ static BOOL sTruncateLongFieldInLogs = YES; } /** - * This function is paired with pingConnection, and performs the keepalive ping in a pthread, + * This function is paired with pingConnection, and performs the checking ping in a pthread, * allowing the thread to be cancelled if it does not respond. */ void pingConnectionTask(void *ptr) @@ -658,62 +654,36 @@ void pingConnectionTask(void *ptr) } /** - * Restarts a keepalive to fire in the future. + * Keeps a connection alive by running a ping. + * This method is called every ten seconds and spawns a thread which determines + * whether or not it should perform a ping. */ -- (void)startKeepAliveTimer +- (void)keepAlive:(NSTimer *)theTimer { - // Ensure keepalives are started on the main thread, as otherwise thread termination kills the timer - if (![NSThread isMainThread]) { - [self performSelectorOnMainThread:@selector(startKeepAliveTimer) withObject:nil waitUntilDone:NO]; - return; - } - - if (keepAliveTimer) [self stopKeepAliveTimer]; - if (!mConnected) return; - - double interval = keepAliveInterval; - if (interval <= 1) interval = 1.0; - - if (useKeepAlive) { - keepAliveTimer = [NSTimer - scheduledTimerWithTimeInterval:interval - target:self - selector:@selector(keepAlive:) - userInfo:nil - repeats:NO]; - [keepAliveTimer retain]; - } -} + // Do nothing if not connected or if keepalive is disabled + if (!mConnected || !useKeepAlive) return; -/** - * Stops a keepalive if one is set for the future, and kills any existing keepalive pings. - */ -- (void)stopKeepAliveTimer -{ - // Stop keepalives on the main thread to avoid memory issues - if (![NSThread isMainThread]) { - [self performSelectorOnMainThread:@selector(stopKeepAliveTimer) withObject:nil waitUntilDone:NO]; + // Check to see whether a ping is required. First, compare the last query + // and keepalive times against the keepalive interval. + // Compare against interval-1 to allow default keepalive intervals to repeat + // at the correct intervals (eg no timer interval delay). + double timeConnected = [self timeConnected]; + if (timeConnected - lastQueryExecutedAtTime < keepAliveInterval - 1 + || timeConnected - lastKeepAliveTime < keepAliveInterval - 1) + { return; } - if (keepAliveThread != NULL) pthread_cancel(keepAliveThread), keepAliveThread = NULL; - - if (!keepAliveTimer) return; - [keepAliveTimer invalidate]; - [keepAliveTimer release]; - keepAliveTimer = nil; -} + // Attempt to get a query lock, but release it to ensure the connection isn't locked + // by a background ping. + if (![queryLock tryLock]) return; + [queryLock unlock]; -/** - * Keeps a connection alive by running a ping. - */ -- (void)keepAlive:(NSTimer *)theTimer -{ - if (!mConnected) return; + // Store the ping time + lastKeepAliveTime = timeConnected; [NSThread detachNewThreadSelector:@selector(threadedKeepAlive) toTarget:self withObject:nil]; - [self startKeepAliveTimer]; } /** @@ -729,19 +699,18 @@ void pingConnectionTask(void *ptr) NSInteger pingTimeout = 30; if (connectionTimeout > 0 && connectionTimeout < pingTimeout) pingTimeout = connectionTimeout; - // Attempt to get a query lock, but release it to ensure the connection isn't locked - // by a background ping. - if (![queryLock tryLock]) return; - [queryLock unlock]; - // Create a pthread for the actual keepalive - pthread_create(&keepAliveThread, NULL, (void *)&performThreadedKeepAlive, (void *)mConnection); + pthread_attr_t attr; + pthread_attr_init(&attr); + pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); + pthread_create(&keepAliveThread, &attr, (void *)&performThreadedKeepAlive, (void *)mConnection); // Give the connection time to respond, but force a timeout after the ping timeout // if the thread hasn't already closed itself. sleep(pingTimeout); pthread_cancel(keepAliveThread); keepAliveThread = NULL; + pthread_attr_destroy(&attr); } /** @@ -761,10 +730,7 @@ void performThreadedKeepAlive(void *ptr) connectionThreadId = mConnection->thread_id; connectionStartTime = mach_absolute_time(); [self fetchMaxAllowedPacket]; - - [self stopKeepAliveTimer]; - [self startKeepAliveTimer]; - + if (delegate && [delegate respondsToSelector:@selector(onReconnectShouldUseEncoding:)]) { [self queryString:[NSString stringWithFormat:@"/*!40101 SET NAMES '%@' */", [NSString stringWithString:[delegate onReconnectShouldUseEncoding:self]]]]; if (delegate && [delegate respondsToSelector:@selector(connectionEncodingViaLatin1:)]) { @@ -1123,8 +1089,6 @@ void performThreadedKeepAlive(void *ptr) { if (!mConnected) return NO; - [self stopKeepAliveTimer]; - if (![self checkConnection]) return NO; // Here we should throw an exception, impossible to select a databse if the string is indeed a nil pointer @@ -1135,7 +1099,6 @@ void performThreadedKeepAlive(void *ptr) [queryLock lock]; if (0 == mysql_select_db(mConnection, theDBName)) { [queryLock unlock]; - [self startKeepAliveTimer]; return YES; } @@ -1384,8 +1347,6 @@ void performThreadedKeepAlive(void *ptr) return nil; } - (void)(*stopKeepAliveTimerPtr)(self, stopKeepAliveTimerSEL); - // Inform the delegate about the query if logging is enabled and delegate responds to willQueryString:connection: if (delegateQueryLogging && delegateResponseToWillQueryString) { [delegate willQueryString:query connection:self]; @@ -1547,8 +1508,6 @@ void performThreadedKeepAlive(void *ptr) if (queryResultCode & delegateResponseToWillQueryString) [delegate queryGaveError:lastQueryErrorMessage connection:self]; - (void)(*startKeepAliveTimerPtr)(self, startKeepAliveTimerSEL, YES); - if (!theResult) return nil; return [theResult autorelease]; } @@ -1698,7 +1657,7 @@ void performThreadedKeepAlive(void *ptr) // Ensure the unlock occurs on the main thread if (![NSThread isMainThread]) { - [self performSelectorOnMainThread:@selector(unlockConnection) withObject:nil waitUntilDone:NO]; + [self performSelectorOnMainThread:@selector(unlockConnection) withObject:nil waitUntilDone:YES]; return; } @@ -1732,12 +1691,8 @@ void performThreadedKeepAlive(void *ptr) MCPResult *theResult = nil; MYSQL_RES *theResPtr; - [self stopKeepAliveTimer]; - if (![self checkConnection]) return [[[MCPResult alloc] init] autorelease]; - [self startKeepAliveTimer]; - [queryLock lock]; if ((dbsName == nil) || ([dbsName isEqualToString:@""])) { if (theResPtr = mysql_list_dbs(mConnection, NULL)) { @@ -1789,11 +1744,7 @@ void performThreadedKeepAlive(void *ptr) MCPResult *theResult = nil; MYSQL_RES *theResPtr; - [self stopKeepAliveTimer]; - if (![self checkConnection]) return [[[MCPResult alloc] init] autorelease]; - - [self startKeepAliveTimer]; [queryLock lock]; if ((tablesName == nil) || ([tablesName isEqualToString:@""])) { @@ -2527,6 +2478,8 @@ void performThreadedKeepAlive(void *ptr) [connectionProxy disconnect]; } + [keepAliveTimer invalidate]; + [keepAliveTimer release]; [queryLock release]; if (lastQueryErrorMessage) [lastQueryErrorMessage release]; if (connectionHost) [connectionHost release]; -- cgit v1.2.3