From f741be75bc3f3d0964942a66d0fafc7ba0c62208 Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 11 May 2017 19:06:39 +0200 Subject: Expand a lock in the connection keepalive code, because it still allowed a zombie call --- .../SPMySQLConnection Categories/Ping & KeepAlive.m | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/Frameworks/SPMySQLFramework/Source/SPMySQLConnection Categories/Ping & KeepAlive.m b/Frameworks/SPMySQLFramework/Source/SPMySQLConnection Categories/Ping & KeepAlive.m index d46b5552..8728bf3f 100644 --- a/Frameworks/SPMySQLFramework/Source/SPMySQLConnection Categories/Ping & KeepAlive.m +++ b/Frameworks/SPMySQLFramework/Source/SPMySQLConnection Categories/Ping & KeepAlive.m @@ -85,9 +85,9 @@ if(keepAliveThread) { NSLog(@"warning: overwriting existing keepAliveThread: %@, results may be unpredictable!",keepAliveThread); } + keepAliveThread = [NSThread currentThread]; } - keepAliveThread = [NSThread currentThread]; [keepAliveThread setName:[NSString stringWithFormat:@"SPMySQL connection keepalive monitor thread (id=%p)", self]]; // If the maximum number of ping failures has been reached, determine whether to reconnect. @@ -282,7 +282,24 @@ void _pingThreadCleanup(void *pingDetails) if (keepAliveThread) { // Mark the thread as cancelled - [keepAliveThread cancel]; + @synchronized(self) { + // the synchronized is neccesary here, because we don't retain keepAliveThread. + // If it were ommitted, for example this could happen: + // + // this thread keepalive thread + // -------------- ----------------- + // 1 fetch value of keepAliveThread to register + // 2 keepAliveThread = nil + // 3 [[NSThread currentThread] release] + // 4 objc_msgSend() <-- invalid memory accessed + // + // With synchronized we are guaranteed to either message nil or block the keepAliveThread from exiting + // (and thus releasing the NSThread object) until this call finishes. + // + // We can omit it in the other 2 cases, since keepAliveThread is already volatile and we are only + // checking for NULL, not dereferencing it. + [keepAliveThread cancel]; + } // Wait inside a time limit of ten seconds for it to exit uint64_t threadCancelStartTime_t = mach_absolute_time(); -- cgit v1.2.3