diff --git a/ChangeLog b/ChangeLog index ecc13dd81..9dfa174ec 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2010-08-30 Richard Frith-Macdonald + + * Source/NSThread.m: try to make inter-thread notification via + pipe more robust. + * Source/NSTask.m: make windows thread waiting for task into an + NSThread so that any thread-dependent code can work. + 2010-08-25 Jonathan Gillaspie * Source/Objective2/sync.m: Removed weak declarations diff --git a/Source/NSTask.m b/Source/NSTask.m index 8e41febeb..80fe7c584 100644 --- a/Source/NSTask.m +++ b/Source/NSTask.m @@ -1009,35 +1009,43 @@ GSPrivateCheckTasks() */ static DWORD WINAPI _threadFunction(LPVOID t) { - DWORD milliseconds = 60000; - int taskId = [(NSTask*)t processIdentifier]; + DWORD milliseconds = 60000; + int taskId; + NSConcreteWindowsTask *task; - for (;;) + GSRegisterCurrentThread(); + taskId = [(NSTask*)t processIdentifier]; + [tasksLock lock]; + task = (NSConcreteWindowsTask*)NSMapGet(activeTasks, + (void*)(intptr_t) taskId); + [task retain]; + [tasksLock unlock]; + if (task != nil) { - NSConcreteWindowsTask *task; - - [tasksLock lock]; - task = (NSConcreteWindowsTask*)NSMapGet(activeTasks, - (void*)(intptr_t) taskId); - IF_NO_GC([[task retain] autorelease];) - [tasksLock unlock]; - if (task == nil) + for (;;) { - return 0; // Task gone away. - } - switch (WaitForSingleObject(task->procInfo.hProcess, milliseconds)) - { - case WAIT_OBJECT_0: - handleSignal(0); // Signal child process state change. - return 0; + NSConcreteWindowsTask *present; - case WAIT_TIMEOUT: - break; // Timeout ... retry - - default: - return 0; // Error ... stop watching. + [tasksLock lock]; + present = (NSConcreteWindowsTask*)NSMapGet(activeTasks, + (void*)(intptr_t) taskId); + [tasksLock unlock]; + if (present == nil) + { + [task release]; + break; // Task gone away. + } + if (WaitForSingleObject(task->procInfo.hProcess, milliseconds) + != WAIT_TIMEOUT) + { + [task release]; + handleSignal(0); // Signal child process state change. + break; + } } } + GSUnregisterCurrentThread(); + return 0; } diff --git a/Source/NSThread.m b/Source/NSThread.m index 77ec1d918..47c121261 100644 --- a/Source/NSThread.m +++ b/Source/NSThread.m @@ -919,10 +919,11 @@ static void *nsthreadLauncher(void* thread) NSLog(@"Set event failed - %@", [NSError _last]); } #else - if (write(outputFd, "0", 1) != 1) - { - NSLog(@"Write to pipe failed - %@", [NSError _last]); - } + /* The write could concievably fail if the pipe is full, but in that + * case we don't care since the other thread should be woken to handle + * reading anyway. + */ + write(outputFd, "0", 1); #endif [lock unlock]; } @@ -967,6 +968,20 @@ static void *nsthreadLauncher(void* thread) [NSException raise: NSInternalInconsistencyException format: @"Failed to get non block flag for perform in thread"]; } + if ((e = fcntl(outputFd, F_GETFL, 0)) >= 0) + { + e |= NBLK_OPT; + if (fcntl(outputFd, F_SETFL, e) < 0) + { + [NSException raise: NSInternalInconsistencyException + format: @"Failed to set non block flag for perform in thread"]; + } + } + else + { + [NSException raise: NSInternalInconsistencyException + format: @"Failed to get non block flag for perform in thread"]; + } } else { @@ -1024,10 +1039,17 @@ static void *nsthreadLauncher(void* thread) #else if (inputFd >= 0) { - if (read(inputFd, &c, 1) != 1) - { - NSLog(@"Read pipe failed - %@", [NSError _last]); - } + char buf[BUFSIZ]; + + /* We don't care how much we read. If there have been multiple + * performers queued then there will be multiple bytes available, + * but we always handle all available performers, so we can also + * read all available bytes. + * The descriptor is non-blocking ... so it's safe to ask for more + * bytes than are available. + */ + while (read(inputFd, buf, sizeof(buf)) > 0) + ; } #endif