From 6e860c202a46fb7f8d01ea6a856fb3e6f7493bca Mon Sep 17 00:00:00 2001 From: Bjorn Winckler Date: Fri, 10 Apr 2009 18:33:02 +0200 Subject: [PATCH] Beware exceptions when processing input Added comment on the dangers of exceptions being raised when processing input in the frontend. Shuffled the exception handling around in the vim controller. --- src/MacVim/MMAppController.m | 8 +++- src/MacVim/MMVimController.m | 98 ++++++++++++++++++++++---------------------- 2 files changed, 56 insertions(+), 50 deletions(-) diff --git a/src/MacVim/MMAppController.m b/src/MacVim/MMAppController.m index de6405a4..2dbefa85 100644 --- a/src/MacVim/MMAppController.m +++ b/src/MacVim/MMAppController.m @@ -2140,6 +2140,10 @@ fsEventCallback(ConstFSEventStreamRef streamRef, return; } + // NOTE: Be _very_ careful that no exceptions can be raised between here + // and the point at which 'processingFlag' is reset. Otherwise the above + // test could end up always failing and no input queues would ever be + // processed! processingFlag = 1; // NOTE: New input may arrive while we're busy processing; we deal with @@ -2158,7 +2162,7 @@ fsEventCallback(ConstFSEventStreamRef streamRef, for (i = 0; i < count; ++i) { MMVimController *vc = [vimControllers objectAtIndex:i]; if (ukey == [vc identifier]) { - [vc processInputQueue:[queues objectForKey:key]]; + [vc processInputQueue:[queues objectForKey:key]]; // !exceptions break; } } @@ -2169,7 +2173,7 @@ fsEventCallback(ConstFSEventStreamRef streamRef, for (i = 0; i < count; ++i) { MMVimController *vc = [cachedVimControllers objectAtIndex:i]; if (ukey == [vc identifier]) { - [vc processInputQueue:[queues objectForKey:key]]; + [vc processInputQueue:[queues objectForKey:key]]; // !exceptions break; } } diff --git a/src/MacVim/MMVimController.m b/src/MacVim/MMVimController.m index 8928d4c7..3f677aa4 100644 --- a/src/MacVim/MMVimController.m +++ b/src/MacVim/MMVimController.m @@ -436,8 +436,15 @@ static BOOL isUnsafeMessage(int msgid); { if (!isInitialized) return; - [self doProcessInputQueue:queue]; - [windowController processInputQueueDidFinish]; + // NOTE: This method must not raise any exceptions (see comment in the + // calling method). + @try { + [self doProcessInputQueue:queue]; + [windowController processInputQueueDidFinish]; + } + @catch (NSException *ex) { + NSLog(@"[%s] Caught exception (pid=%d): %@", _cmd, pid, ex); + } } - (NSToolbarItem *)toolbar:(NSToolbar *)theToolbar @@ -472,55 +479,50 @@ static BOOL isUnsafeMessage(int msgid); { NSMutableArray *delayQueue = nil; - @try { - unsigned i, count = [queue count]; - if (count % 2) { - NSLog(@"WARNING: Uneven number of components (%d) in command " - "queue. Skipping...", count); - return; - } + unsigned i, count = [queue count]; + if (count % 2) { + NSLog(@"WARNING: Uneven number of components (%d) in command " + "queue. Skipping...", count); + return; + } - //NSLog(@"======== %s BEGIN ========", _cmd); - for (i = 0; i < count; i += 2) { - NSData *value = [queue objectAtIndex:i]; - NSData *data = [queue objectAtIndex:i+1]; - - int msgid = *((int*)[value bytes]); - //NSLog(@"%s%s", _cmd, MessageStrings[msgid]); - - BOOL inDefaultMode = [[[NSRunLoop currentRunLoop] currentMode] - isEqual:NSDefaultRunLoopMode]; - if (!inDefaultMode && isUnsafeMessage(msgid)) { - // NOTE: Because we may be listening to DO messages in "event - // tracking mode" we have to take extra care when doing things - // like releasing view items (and other Cocoa objects). - // Messages that may be potentially "unsafe" are delayed until - // the run loop is back to default mode at which time they are - // safe to call again. - // A problem with this approach is that it is hard to - // classify which messages are unsafe. As a rule of thumb, if - // a message may release an object used by the Cocoa framework - // (e.g. views) then the message should be considered unsafe. - // Delaying messages may have undesired side-effects since it - // means that messages may not be processed in the order Vim - // sent them, so beware. - if (!delayQueue) - delayQueue = [NSMutableArray array]; - - //NSLog(@"Adding unsafe message '%s' to delay queue (mode=%@)", - // MessageStrings[msgid], - // [[NSRunLoop currentRunLoop] currentMode]); - [delayQueue addObject:value]; - [delayQueue addObject:data]; - } else { - [self handleMessage:msgid data:data]; - } + //NSLog(@"======== %s BEGIN ========", _cmd); + for (i = 0; i < count; i += 2) { + NSData *value = [queue objectAtIndex:i]; + NSData *data = [queue objectAtIndex:i+1]; + + int msgid = *((int*)[value bytes]); + //NSLog(@"%s%s", _cmd, MessageStrings[msgid]); + + BOOL inDefaultMode = [[[NSRunLoop currentRunLoop] currentMode] + isEqual:NSDefaultRunLoopMode]; + if (!inDefaultMode && isUnsafeMessage(msgid)) { + // NOTE: Because we may be listening to DO messages in "event + // tracking mode" we have to take extra care when doing things + // like releasing view items (and other Cocoa objects). + // Messages that may be potentially "unsafe" are delayed until + // the run loop is back to default mode at which time they are + // safe to call again. + // A problem with this approach is that it is hard to + // classify which messages are unsafe. As a rule of thumb, if + // a message may release an object used by the Cocoa framework + // (e.g. views) then the message should be considered unsafe. + // Delaying messages may have undesired side-effects since it + // means that messages may not be processed in the order Vim + // sent them, so beware. + if (!delayQueue) + delayQueue = [NSMutableArray array]; + + //NSLog(@"Adding unsafe message '%s' to delay queue (mode=%@)", + // MessageStrings[msgid], + // [[NSRunLoop currentRunLoop] currentMode]); + [delayQueue addObject:value]; + [delayQueue addObject:data]; + } else { + [self handleMessage:msgid data:data]; } - //NSLog(@"======== %s END ========", _cmd); - } - @catch (NSException *e) { - NSLog(@"Exception caught whilst processing command queue: %@", e); } + //NSLog(@"======== %s END ========", _cmd); if (delayQueue) { //NSLog(@" Flushing delay queue (%d items)", [delayQueue count]/2); -- 2.11.4.GIT