Permalink
Browse files

Don't ignore SIGCHLD (fix automatic updating)

Ignoring SIGCHLD caused problems with automatic updating (Sparkle) since
it uses popen() (and hence implicitly uses wait4()) to unpack archives.
Now that SIGCHLD is no longer ignored we have to reap child processes
after exiting a Vim process as well as when MacVim is about to
terminate.
  • Loading branch information...
b4winckler committed Jan 8, 2009
1 parent 1db4b79 commit e648d3b0111433c1001aebced611d8bced9653f2
Showing with 76 additions and 19 deletions.
  1. +1 −0 src/MacVim/MMAppController.h
  2. +75 −19 src/MacVim/MMAppController.m
@@ -28,6 +28,7 @@
NSMutableArray *cachedVimControllers;
int preloadPid;
BOOL shouldActivateWhenNextWindowOpens;
+ int numChildProcesses;
#if (MAC_OS_X_VERSION_MAX_ALLOWED > MAC_OS_X_VERSION_10_4)
FSEventStreamRef fsEventStream;
@@ -82,8 +82,6 @@
#pragma options align=reset
-static int executeInLoginShell(NSString *path, NSArray *args);
-
@interface MMAppController (MMServices)
- (void)openSelection:(NSPasteboard *)pboard userData:(NSString *)userData
@@ -125,6 +123,8 @@ - (void)startWatchingVimDir;
- (void)stopWatchingVimDir;
- (void)handleFSEvent;
- (void)loadDefaultFont;
+- (int)executeInLoginShell:(NSString *)path arguments:(NSArray *)args;
+- (void)reapChildProcesses:(id)sender;
#ifdef MM_ENABLE_PLUGINS
- (void)removePlugInMenu;
@@ -151,9 +151,6 @@ @implementation MMAppController
+ (void)initialize
{
- // Avoid zombies (we fork Vim processes which we don't want to wait for).
- signal(SIGCHLD, SIG_IGN);
-
NSDictionary *dict = [NSDictionary dictionaryWithObjectsAndKeys:
[NSNumber numberWithBool:NO], MMNoWindowKey,
[NSNumber numberWithInt:64], MMTabMinWidthKey,
@@ -508,12 +505,16 @@ - (NSApplicationTerminateReply)applicationShouldTerminate:
if (NSTerminateNow == reply) {
e = [vimControllers objectEnumerator];
id vc;
- while ((vc = [e nextObject]))
+ while ((vc = [e nextObject])) {
+ //NSLog(@"Terminate pid=%d", [vc pid]);
[vc sendMessage:TerminateNowMsgID data:nil];
+ }
e = [cachedVimControllers objectEnumerator];
- while ((vc = [e nextObject]))
+ while ((vc = [e nextObject])) {
+ //NSLog(@"Terminate pid=%d (cached)", [vc pid]);
[vc sendMessage:TerminateNowMsgID data:nil];
+ }
// If a Vim process is being preloaded as we quit we have to forcibly
// kill it since we have not established a connection yet.
@@ -564,6 +565,32 @@ - (void)applicationWillTerminate:(NSNotification *)notification
}
[NSApp setDelegate:nil];
+
+ // Try to wait for all child processes to avoid leaving zombies behind (but
+ // don't wait around for too long).
+ NSDate *timeOutDate = [NSDate dateWithTimeIntervalSinceNow:2];
+ while ([timeOutDate timeIntervalSinceNow] > 0) {
+ [self reapChildProcesses:nil];
+ if (numChildProcesses <= 0)
+ break;
+
+ //NSLog(@"%d processes still left, sleep a bit...", numChildProcesses);
+
+ // Run in NSConnectionReplyMode while waiting instead of calling e.g.
+ // usleep(). Otherwise incoming messages may clog up the DO queues and
+ // the outgoing TerminateNowMsgID sent earlier never reaches the Vim
+ // process.
+ // This has at least one side-effect, namely we may receive the
+ // annoying "dropping incoming DO message". (E.g. this may happen if
+ // you quickly hit Cmd-n several times in a row and then immediately
+ // press Cmd-q, Enter.)
+ while (CFRunLoopRunInMode((CFStringRef)NSConnectionReplyMode,
+ 0.05, true) == kCFRunLoopRunHandledSource)
+ ; // do nothing
+ }
+
+ if (numChildProcesses > 0)
+ NSLog(@"%d ZOMBIES left behind", numChildProcesses);
}
+ (MMAppController *)sharedInstance
@@ -605,6 +632,14 @@ - (void)removeVimController:(id)controller
if (hide)
[NSApp hide:self];
}
+
+ // There is a small delay before the Vim process actually exits so wait a
+ // little before trying to reap the child process. If the process still
+ // hasn't exited after this wait it won't be reaped until the next time
+ // reapChildProcesses: is called (but this should be harmless).
+ [self performSelector:@selector(reapChildProcesses:)
+ withObject:nil
+ afterDelay:0.1];
}
- (void)windowControllerWillOpen:(MMWindowController *)windowController
@@ -1270,7 +1305,7 @@ - (int)launchVimProcessWithArguments:(NSArray *)args
if (useLoginShell) {
// Run process with a login shell, roughly:
// echo "exec Vim -g -f args" | ARGV0=-`basename $SHELL` $SHELL [-l]
- pid = executeInLoginShell(path, taskArgs);
+ pid = [self executeInLoginShell:path arguments:taskArgs];
} else {
// Run process directly:
// Vim -g -f args
@@ -1734,6 +1769,14 @@ - (void)clearPreloadCacheWithCount:(int)count
n = count;
while (n-- > 0 && [cachedVimControllers count] > 0)
[cachedVimControllers removeObjectAtIndex:0];
+
+ // There is a small delay before the Vim process actually exits so wait a
+ // little before trying to reap the child process. If the process still
+ // hasn't exited after this wait it won't be reaped until the next time
+ // reapChildProcesses: is called (but this should be harmless).
+ [self performSelector:@selector(reapChildProcesses:)
+ withObject:nil
+ afterDelay:0.1];
}
- (void)rebuildPreloadCache
@@ -1919,13 +1962,7 @@ - (void)loadDefaultFont
"may be incomplete)");
}
-@end // MMAppController (Private)
-
-
-
-
- static int
-executeInLoginShell(NSString *path, NSArray *args)
+- (int)executeInLoginShell:(NSString *)path arguments:(NSArray *)args
{
// Start a login shell and execute the command 'path' with arguments 'args'
// in the shell. This ensures that user environment variables are set even
@@ -1997,10 +2034,6 @@ - (void)loadDefaultFont
} else if (pid == 0) {
// Child process
- // We need to undo our zombie avoidance as Vim waits for and needs
- // the exit statuses of processes it spawns.
- signal(SIGCHLD, SIG_DFL);
-
if (close(ds[1]) == -1) exit(255);
if (dup2(ds[0], 0) == -1) exit(255);
@@ -2024,7 +2057,30 @@ - (void)loadDefaultFont
if (write(ds[1], [input UTF8String], bytes) != bytes) return -1;
if (close(ds[1]) == -1) return -1;
+
+ ++numChildProcesses;
+ //NSLog(@"new process pid=%d (count=%d)", pid, numChildProcesses);
}
return pid;
}
+
+- (void)reapChildProcesses:(id)sender
+{
+ // NOTE: numChildProcesses (currently) only counts the number of Vim
+ // processes that have been started with executeInLoginShell::. If other
+ // processes are spawned this code may need to be adjusted (or
+ // numChildProcesses needs to be incremented when such a process is
+ // started).
+ while (numChildProcesses > 0) {
+ int status = 0;
+ int pid = waitpid(-1, &status, WNOHANG);
+ if (pid <= 0)
+ break;
+
+ //NSLog(@"WAIT for pid=%d complete", pid);
+ --numChildProcesses;
+ }
+}
+
+@end // MMAppController (Private)

0 comments on commit e648d3b

Please sign in to comment.