From 4ee0658e6ae4326c24db9b344b4dc8dc7bdf9263 Mon Sep 17 00:00:00 2001 From: Robert Sesek Date: Wed, 3 Jul 2013 19:14:09 -0400 Subject: [PATCH] Write BSProtocolThreadInvoker to replace ThreadSafeDelegate. This class protects against reentrancy, which happens when AppKit uses WebKit on the main thread for NSAttributedString. --- MacGDBp.xcodeproj/project.pbxproj | 12 +- ...afeDeleage.h => BSProtocolThreadInvoker.h} | 9 +- Source/BSProtocolThreadInvoker.m | 200 ++++++++++++++++++ Source/MessageQueue.h | 4 +- Source/MessageQueue.m | 7 +- Source/ThreadSafeDeleage.m | 86 -------- 6 files changed, 219 insertions(+), 99 deletions(-) rename Source/{ThreadSafeDeleage.h => BSProtocolThreadInvoker.h} (67%) create mode 100644 Source/BSProtocolThreadInvoker.m delete mode 100644 Source/ThreadSafeDeleage.m diff --git a/MacGDBp.xcodeproj/project.pbxproj b/MacGDBp.xcodeproj/project.pbxproj index de7fa27..330a3ca 100644 --- a/MacGDBp.xcodeproj/project.pbxproj +++ b/MacGDBp.xcodeproj/project.pbxproj @@ -24,7 +24,7 @@ 1E416FF90D36F821009A53A2 /* MainMenu.xib in Resources */ = {isa = PBXBuildFile; fileRef = 1E416FF60D36F821009A53A2 /* MainMenu.xib */; }; 1E42F1D70F53317B008412DB /* dsa_pub.pem in Resources */ = {isa = PBXBuildFile; fileRef = 1E42F1D60F53317B008412DB /* dsa_pub.pem */; }; 1E4C7AF90DA401C7000A9DC7 /* BreakpointManager.m in Sources */ = {isa = PBXBuildFile; fileRef = 1E4C7AF80DA401C7000A9DC7 /* BreakpointManager.m */; }; - 1E5C32AA177296DF00F4377B /* ThreadSafeDeleage.m in Sources */ = {isa = PBXBuildFile; fileRef = 1E5C32A9177296DF00F4377B /* ThreadSafeDeleage.m */; }; + 1E5C32AA177296DF00F4377B /* BSProtocolThreadInvoker.m in Sources */ = {isa = PBXBuildFile; fileRef = 1E5C32A9177296DF00F4377B /* BSProtocolThreadInvoker.m */; }; 1E67E6FD0F3C052000E68F1B /* PreferencesPathsArrayController.m in Sources */ = {isa = PBXBuildFile; fileRef = 1E67E6FC0F3C052000E68F1B /* PreferencesPathsArrayController.m */; }; 1E6B5947116106FE001189D2 /* LoggingController.m in Sources */ = {isa = PBXBuildFile; fileRef = 1E6B5946116106FE001189D2 /* LoggingController.m */; }; 1E6B594C11610993001189D2 /* Log.xib in Resources */ = {isa = PBXBuildFile; fileRef = 1E6B594A11610993001189D2 /* Log.xib */; }; @@ -98,8 +98,8 @@ 1E42F1D60F53317B008412DB /* dsa_pub.pem */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = dsa_pub.pem; sourceTree = ""; }; 1E4C7AF70DA401C7000A9DC7 /* BreakpointManager.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = BreakpointManager.h; path = Source/BreakpointManager.h; sourceTree = ""; }; 1E4C7AF80DA401C7000A9DC7 /* BreakpointManager.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = BreakpointManager.m; path = Source/BreakpointManager.m; sourceTree = ""; }; - 1E5C32A8177296DF00F4377B /* ThreadSafeDeleage.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ThreadSafeDeleage.h; path = Source/ThreadSafeDeleage.h; sourceTree = ""; }; - 1E5C32A9177296DF00F4377B /* ThreadSafeDeleage.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = ThreadSafeDeleage.m; path = Source/ThreadSafeDeleage.m; sourceTree = ""; }; + 1E5C32A8177296DF00F4377B /* BSProtocolThreadInvoker.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = BSProtocolThreadInvoker.h; path = Source/BSProtocolThreadInvoker.h; sourceTree = ""; }; + 1E5C32A9177296DF00F4377B /* BSProtocolThreadInvoker.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = BSProtocolThreadInvoker.m; path = Source/BSProtocolThreadInvoker.m; sourceTree = ""; }; 1E67E6FB0F3C052000E68F1B /* PreferencesPathsArrayController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = PreferencesPathsArrayController.h; path = Source/PreferencesPathsArrayController.h; sourceTree = ""; }; 1E67E6FC0F3C052000E68F1B /* PreferencesPathsArrayController.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = PreferencesPathsArrayController.m; path = Source/PreferencesPathsArrayController.m; sourceTree = ""; }; 1E6B5945116106FE001189D2 /* LoggingController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = LoggingController.h; path = Source/LoggingController.h; sourceTree = ""; }; @@ -320,8 +320,8 @@ 1EEBE841176FEA80003622C3 /* MessageQueue.m */, 1EEBE843176FFE04003622C3 /* ProtocolClient.h */, 1EEBE844176FFE04003622C3 /* ProtocolClient.m */, - 1E5C32A8177296DF00F4377B /* ThreadSafeDeleage.h */, - 1E5C32A9177296DF00F4377B /* ThreadSafeDeleage.m */, + 1E5C32A8177296DF00F4377B /* BSProtocolThreadInvoker.h */, + 1E5C32A9177296DF00F4377B /* BSProtocolThreadInvoker.m */, ); name = Protocol; sourceTree = ""; @@ -491,7 +491,7 @@ 1E109019136DD92D002E34E0 /* StripLineBreaksValueTransformer.m in Sources */, 1EEBE842176FEA80003622C3 /* MessageQueue.m in Sources */, 1EEBE845176FFE04003622C3 /* ProtocolClient.m in Sources */, - 1E5C32AA177296DF00F4377B /* ThreadSafeDeleage.m in Sources */, + 1E5C32AA177296DF00F4377B /* BSProtocolThreadInvoker.m in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/Source/ThreadSafeDeleage.h b/Source/BSProtocolThreadInvoker.h similarity index 67% rename from Source/ThreadSafeDeleage.h rename to Source/BSProtocolThreadInvoker.h index f80dfa0..da6543a 100644 --- a/Source/ThreadSafeDeleage.h +++ b/Source/BSProtocolThreadInvoker.h @@ -16,8 +16,15 @@ #import -@interface ThreadSafeDeleage : NSObject +// BSProtocolThreadInvoker will forward all messages that are part of |protocol| +// to its target |object| on the specified |thread| in the optional modes. This +// allows a client to hold this as a delegate and write thread-safe code with +// minimal work. The class also protects against the target |object| reentering +// itself; if it or something it calls runs a nested run loop, the messages +// will be queued until it would no longer reenter the object. +@interface BSProtocolThreadInvoker : NSObject +// The target object to which messages will be sent. @property(readonly, atomic) NSObject* object; - (id)initWithObject:(NSObject*)object diff --git a/Source/BSProtocolThreadInvoker.m b/Source/BSProtocolThreadInvoker.m new file mode 100644 index 0000000..24c04e4 --- /dev/null +++ b/Source/BSProtocolThreadInvoker.m @@ -0,0 +1,200 @@ +/* + * MacGDBp + * Copyright (c) 2013, Blue Static + * + * This program is free software; you can redistribute it and/or modify it under the terms of the GNU + * General Public License as published by the Free Software Foundation; either version 2 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without + * even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License along with this program; if not, + * write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#import "BSProtocolThreadInvoker.h" + +@interface BSProtocolThreadInvoker (Private) +// Installs a run loop observer on the target thread that is used to start +// dispatching messages again after falling out of a nested run loop. +// *MUST* be called on the target thread. +- (void)addRunLoopObserver; + +// Removes the observer added with |-addRunLoopObserver|. Can be called on any +// thread since CFRunLoop is threadsafe. +- (void)removeRunLoopObserver; + +// Enqueues the |invocation| for execution, and dequeues the first if not called +// reentrantly. +// *MUST* be called on the target thread. +- (void)dispatchInvocation:(NSInvocation*)invocation; + +// Callback for the run loop whenever it begins a new pass. This will schedule +// work if any was previously deferred due to reentrancy protection. +- (void)observedRunLoopEnter; +@end + +@implementation BSProtocolThreadInvoker { + // The fully qualified target of the invocation. + NSObject* _object; + Protocol* _protocol; + NSThread* _thread; + CFRunLoopRef _runLoop; + NSArray* _modes; + + // If executing an invocation from |-dispatchInvocation:|. Protects against + // reentering the target. + BOOL _isDispatching; + + // The queue of work to be executed. Enqueues in |-dispatchInvocation:|. + NSMutableArray* _invocations; + + CFRunLoopObserverRef _observer; +} + +@synthesize object = _object; + +- (id)initWithObject:(NSObject*)object + protocol:(Protocol*)protocol + thread:(NSThread*)thread +{ + return [self initWithObject:object + protocol:protocol + thread:thread + modes:@[ NSRunLoopCommonModes ]]; +} + +- (id)initWithObject:(NSObject*)object + protocol:(Protocol*)protocol + thread:(NSThread*)thread + modes:(NSArray*)runLoopModes +{ + if ((self = [super init])) { + _object = object; + _protocol = protocol; + _thread = thread; + _modes = [runLoopModes retain]; + _invocations = [[NSMutableArray alloc] init]; + + [self performSelector:@selector(addRunLoopObserver) + onThread:_thread + withObject:nil + waitUntilDone:NO + modes:_modes]; + } + return self; +} + +- (void)dealloc +{ + [self removeRunLoopObserver]; + [_modes release]; + [_invocations release]; + [super dealloc]; +} + +- (BOOL)conformsToProtocol:(Protocol*)protocol +{ + return [_protocol isEqual:protocol]; +} + +- (NSMethodSignature*)methodSignatureForSelector:(SEL)aSelector +{ + if (!_object) + return [_protocol methodSignatureForSelector:aSelector]; + return [_object methodSignatureForSelector:aSelector]; +} + +- (BOOL)respondsToSelector:(SEL)aSelector +{ + if (!_object) + return [_protocol respondsToSelector:aSelector]; + return [_object respondsToSelector:aSelector]; +} + +- (void)forwardInvocation:(NSInvocation*)invocation +{ + if ([_object respondsToSelector:[invocation selector]]) { + [invocation retainArguments]; + [self performSelector:@selector(dispatchInvocation:) + onThread:_thread + withObject:invocation + waitUntilDone:NO + modes:_modes]; + } +} + +// Private ///////////////////////////////////////////////////////////////////// + +- (void)addRunLoopObserver +{ + assert([NSThread currentThread] == _thread); + _runLoop = CFRunLoopGetCurrent(); + + BSProtocolThreadInvoker* __block weakSelf = self; + _observer = CFRunLoopObserverCreateWithHandler( + kCFAllocatorDefault, + kCFRunLoopEntry, + TRUE, // Repeats. + 0, // Order. + ^(CFRunLoopObserverRef observer, CFRunLoopActivity activity) { + [weakSelf observedRunLoopEnter]; + }); + for (NSString* mode in _modes) + CFRunLoopAddObserver(_runLoop, _observer, (CFStringRef)mode); +} + +- (void)removeRunLoopObserver +{ + for (NSString* mode in _modes) + CFRunLoopRemoveObserver(_runLoop, _observer, (CFStringRef)mode); + CFRelease(_observer); +} + +- (void)dispatchInvocation:(NSInvocation*)invocation +{ + // |invocation| will be nil if dispatch was requested after entering a new + // pass of the run loop, to process deferred work. + if (invocation) + [_invocations addObject:invocation]; + + // Protect the target object from reentering itself. This work will be + // rescheduled when another run loop starts (including falling out of a + // nested loop and starting a new pass through a lower loop). + if (_isDispatching) + return; + + _isDispatching = YES; + + // Dequeue only one item. If multiple items are present, the next pass through + // the run loop will schedule another dispatch via |-observedRunLoopEnter|. + invocation = [_invocations objectAtIndex:0]; + [invocation invokeWithTarget:_object]; + [_invocations removeObjectAtIndex:0]; + + _isDispatching = NO; +} + +- (void)observedRunLoopEnter +{ + // Don't do anything if there's nothing to do. + if ([_invocations count] == 0) + return; + + // If this nested run loop is still executing from within + // |-dispatchInvocation:|, continue to wait for a the nested loop to exit. + if (_isDispatching) + return; + + // A run loop has started running outside of |-dispatchInvocation:|, so + // schedule work to be done again. + [self performSelector:@selector(dispatchInvocation:) + onThread:_thread + withObject:nil + waitUntilDone:NO + modes:_modes]; +} + +@end diff --git a/Source/MessageQueue.h b/Source/MessageQueue.h index 2e62288..16961d7 100644 --- a/Source/MessageQueue.h +++ b/Source/MessageQueue.h @@ -16,7 +16,7 @@ #import -#import "ThreadSafeDeleage.h" +#import "BSProtocolThreadInvoker.h" @protocol MessageQueueDelegate; @@ -40,7 +40,7 @@ NSMutableArray* _queue; // The delegate for this class. - ThreadSafeDeleage* _delegate; + BSProtocolThreadInvoker* _delegate; // The socket that listens for new incoming connections. CFSocketRef _socket; diff --git a/Source/MessageQueue.m b/Source/MessageQueue.m index 8ada000..0a657e3 100644 --- a/Source/MessageQueue.m +++ b/Source/MessageQueue.m @@ -88,11 +88,10 @@ static void MessageQueueWriteEvent(CFWriteStreamRef stream, if ((self = [super init])) { _port = port; _queue = [[NSMutableArray alloc] init]; - _delegate = (ThreadSafeDeleage*) - [[ThreadSafeDeleage alloc] initWithObject:delegate + _delegate = (BSProtocolThreadInvoker*) + [[BSProtocolThreadInvoker alloc] initWithObject:delegate protocol:@protocol(MessageQueueDelegate) - thread:[NSThread currentThread] - modes:@[ NSDefaultRunLoopMode ]]; + thread:[NSThread currentThread]]; } return self; } diff --git a/Source/ThreadSafeDeleage.m b/Source/ThreadSafeDeleage.m deleted file mode 100644 index 4c0493f..0000000 --- a/Source/ThreadSafeDeleage.m +++ /dev/null @@ -1,86 +0,0 @@ -/* - * MacGDBp - * Copyright (c) 2013, Blue Static - * - * This program is free software; you can redistribute it and/or modify it under the terms of the GNU - * General Public License as published by the Free Software Foundation; either version 2 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without - * even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License along with this program; if not, - * write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA - */ - -#import "ThreadSafeDeleage.h" - -@implementation ThreadSafeDeleage { - NSObject* _object; - Protocol* _protocol; - NSThread* _thread; - NSArray* _modes; -} - -@synthesize object = _object; - -- (id)initWithObject:(NSObject*)object - protocol:(Protocol*)protocol - thread:(NSThread*)thread { - return [self initWithObject:object - protocol:protocol - thread:thread - modes:@[ NSRunLoopCommonModes ]]; -} - -- (id)initWithObject:(NSObject*)object - protocol:(Protocol*)protocol - thread:(NSThread*)thread - modes:(NSArray*)runLoopModes { - if ((self = [super init])) { - _object = object; - _protocol = protocol; - _thread = thread; - _modes = [runLoopModes retain]; - } - return self; -} - -- (void)dealloc { - [_modes release]; - [super dealloc]; -} - -- (BOOL)conformsToProtocol:(Protocol*)protocol { - return [_protocol isEqual:protocol]; -} - -- (NSMethodSignature*)methodSignatureForSelector:(SEL)aSelector { - if (!_object) - return [_protocol methodSignatureForSelector:aSelector]; - return [_object methodSignatureForSelector:aSelector]; -} - -- (BOOL)respondsToSelector:(SEL)aSelector { - if (!_object) - return [_protocol respondsToSelector:aSelector]; - return [_object respondsToSelector:aSelector]; -} - -- (void)forwardInvocation:(NSInvocation*)invocation { - if ([_object respondsToSelector:[invocation selector]]) { - [invocation retainArguments]; - [self performSelector:@selector(dispatchInvocation:) - onThread:_thread - withObject:invocation - waitUntilDone:NO - modes:_modes]; - } -} - -- (void)dispatchInvocation:(NSInvocation*)invocation { - [invocation invokeWithTarget:_object]; -} - -@end -- 2.22.5