diff --git a/ChangeLog b/ChangeLog index c39cbc51a..c1b06fdcf 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,22 @@ +2009-09-27 Richard Frith-Macdonald + + * Source\win32\GSFileHandle.m: + * Source\win32\NSMessagePort.m: + * Source\win32\GSRunLoopCtxt.m: + * Source\win32\NSStream.m: + * Source\win32\NSMessagePortNameServer.m: + * Source\NSPipe.m: + * Source\NSTask.m: + Fixes to try and improve subtask handling on windows. + 1. change scheme to create all handles as not inheritable, and + just make the handles we want the child to have inheritable while + we are launching the child. + 2. peek the data on pipes and refrain from notifying about data + availability if the pipe doesn't really have data. This is horrid and + means we poll pipes ... but I can see no other way of implementing the + api on windows as I can find no way to get windows to tell use when + data is available. + 2009-09-23 Richard Frith-Macdonald * Source/NSURL.m: OSX compatibility tweaks ... allows initialisation diff --git a/Source/NSPipe.m b/Source/NSPipe.m index 472dc5f4c..7e7e0fd95 100644 --- a/Source/NSPipe.m +++ b/Source/NSPipe.m @@ -87,7 +87,7 @@ HANDLE readh, writeh; saAttr.nLength = sizeof(SECURITY_ATTRIBUTES); - saAttr.bInheritHandle = TRUE; + saAttr.bInheritHandle = FALSE; saAttr.lpSecurityDescriptor = NULL; if (CreatePipe(&readh, &writeh, &saAttr, 0) != 0) diff --git a/Source/NSTask.m b/Source/NSTask.m index c6d9ca26c..fad89f7ae 100644 --- a/Source/NSTask.m +++ b/Source/NSTask.m @@ -1092,6 +1092,10 @@ quotedFromString(NSString *aString) NSDictionary *env; NSMutableArray *toClose; NSFileHandle *hdl; + HANDLE hIn; + HANDLE hOut; + HANDLE hErr; + id last = nil; if (_hasLaunched) { @@ -1163,51 +1167,100 @@ quotedFromString(NSString *aString) start_info.dwFlags |= STARTF_USESTDHANDLES; toClose = [NSMutableArray arrayWithCapacity: 3]; - hdl = [self standardInput]; - if ([hdl isKindOfClass: [NSPipe class]]) - { - hdl = [(NSPipe*)hdl fileHandleForReading]; - [toClose addObject: hdl]; - } - start_info.hStdInput = [hdl nativeHandle]; - hdl = [self standardOutput]; - if ([hdl isKindOfClass: [NSPipe class]]) + if (_standardInput == nil) { - hdl = [(NSPipe*)hdl fileHandleForWriting]; - [toClose addObject: hdl]; + start_info.hStdInput = GetStdHandle(STD_INPUT_HANDLE); } - start_info.hStdOutput = [hdl nativeHandle]; - - hdl = [self standardError]; - if ([hdl isKindOfClass: [NSPipe class]]) + else { - hdl = [(NSPipe*)hdl fileHandleForWriting]; - /* - * If we have the same pipe twice we don't want to close it twice - */ - if ([toClose indexOfObjectIdenticalTo: hdl] == NSNotFound) + hdl = [self standardInput]; + if ([hdl isKindOfClass: [NSPipe class]]) { + hdl = [(NSPipe*)hdl fileHandleForReading]; [toClose addObject: hdl]; } + start_info.hStdInput = [hdl nativeHandle]; } - start_info.hStdError = [hdl nativeHandle]; + hIn = start_info.hStdInput; + + if (_standardOutput == nil) + { + start_info.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE); + } + else + { + hdl = [self standardOutput]; + if ([hdl isKindOfClass: [NSPipe class]]) + { + hdl = [(NSPipe*)hdl fileHandleForWriting]; + [toClose addObject: hdl]; + } + start_info.hStdOutput = [hdl nativeHandle]; + } + hOut = start_info.hStdOutput; + + if (_standardError == nil) + { + start_info.hStdError = GetStdHandle(STD_ERROR_HANDLE); + } + else + { + hdl = [self standardError]; + if ([hdl isKindOfClass: [NSPipe class]]) + { + hdl = [(NSPipe*)hdl fileHandleForWriting]; + /* + * If we have the same pipe twice we don't want to close it twice + */ + if ([toClose indexOfObjectIdenticalTo: hdl] == NSNotFound) + { + [toClose addObject: hdl]; + } + } + start_info.hStdError = [hdl nativeHandle]; + } + hErr = start_info.hStdError; + + /* Make the handles inheritable only temporarily while launching the + * child task. This section must be lock protected so we don't have + * another thread trying to launch at the same time and get handles + * inherited by the wrong threads. + */ + [tasksLock lock]; + SetHandleInformation(hIn, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT); + SetHandleInformation(hOut, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT); + SetHandleInformation(hErr, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT); result = CreateProcessW(wexecutable, w_args, NULL, /* proc attrs */ NULL, /* thread attrs */ 1, /* inherit handles */ -// CREATE_NO_WINDOW|DETACHED_PROCESS|CREATE_UNICODE_ENVIRONMENT, - CREATE_NO_WINDOW|CREATE_UNICODE_ENVIRONMENT, + 0 +// |CREATE_NO_WINDOW +/* One would have thought the the CREATE_NO_WINDOW flag should be used, + * but apparently this breaks for old 16bit applications/tools on XP. + */ + |DETACHED_PROCESS + |CREATE_UNICODE_ENVIRONMENT, envp, /* env block */ (const unichar*)[[self currentDirectoryPath] fileSystemRepresentation], &start_info, &procInfo); - NSZoneFree(NSDefaultMallocZone(), w_args); if (result == 0) { - NSLog(@"Error launching task: %@", lpath); + last = [NSError _last]; + } + NSZoneFree(NSDefaultMallocZone(), w_args); + SetHandleInformation(hIn, HANDLE_FLAG_INHERIT, 0); + SetHandleInformation(hOut, HANDLE_FLAG_INHERIT, 0); + SetHandleInformation(hErr, HANDLE_FLAG_INHERIT, 0); + [tasksLock unlock]; + + if (result == 0) + { + NSLog(@"Error launching task: %@ ... %@", lpath, last); return; } diff --git a/Source/win32/GSFileHandle.m b/Source/win32/GSFileHandle.m index fc976704c..41264ec61 100644 --- a/Source/win32/GSFileHandle.m +++ b/Source/win32/GSFileHandle.m @@ -2003,68 +2003,99 @@ NSString * const GSSOCKSRecvAddr = @"GSSOCKSRecvAddr"; } [self postReadNotification]; } - else if (operation == NSFileHandleDataAvailableNotification) - { - [self postReadNotification]; - } else { - NSMutableData *item; - int length; - int received = 0; - char buf[READ_SIZE]; - - item = [readInfo objectForKey: NSFileHandleNotificationDataItem]; - /* - * We may have a maximum data size set... + /* If this is not a socket or a standard file, we assume it's a pipe + * and therefore we need to check to see if data really is available. */ - if (readMax > 0) - { - length = (unsigned int)readMax - [item length]; - if (length > (int)sizeof(buf)) - { + if (NO == isSocket && NO == isStandardFile) + { + HANDLE h = (HANDLE)_get_osfhandle(descriptor); + DWORD bytes = 0; + + if (PeekNamedPipe(h, 0, 0, 0, &bytes, 0) == 0) + { + DWORD e = GetLastError(); + + if (e != ERROR_BROKEN_PIPE && e != ERROR_HANDLE_EOF) + { + NSLog(@"pipe peek problem %d, %@", e, [NSError _last]); + return; + } + /* In the case of a broken pipe, we fall through so that a read + * attempt is performed allowing higer level code to notice the + * problem and deal with it. + */ + } + else if (bytes == 0) + { + return; // No data available yet. + } + } + + if (operation == NSFileHandleDataAvailableNotification) + { + [self postReadNotification]; + } + else + { + NSMutableData *item; + int length; + int received = 0; + char buf[READ_SIZE]; + + item = [readInfo objectForKey: NSFileHandleNotificationDataItem]; + /* + * We may have a maximum data size set... + */ + if (readMax > 0) + { + length = (unsigned int)readMax - [item length]; + if (length > (int)sizeof(buf)) + { + length = sizeof(buf); + } + } + else + { length = sizeof(buf); } - } - else - { - length = sizeof(buf); - } - received = [self read: buf length: length]; - if (received == 0) - { // Read up to end of file. - [self postReadNotification]; - } - else if (received < 0) - { - if (isSocket && (WSAGetLastError() != WSAEINTR - && WSAGetLastError() != WSAEWOULDBLOCK)) - { - NSString *s; - - s = [NSString stringWithFormat: @"Read attempt failed - %@", - [NSError _last]]; - [readInfo setObject: s forKey: GSFileHandleNotificationError]; + received = [self read: buf length: length]; + if (received == 0) + { // Read up to end of file. [self postReadNotification]; } - else if (!isSocket && (GetLastError() != ERROR_NO_DATA)) - { - NSString *s; - - s = [NSString stringWithFormat: @"Read attempt failed - %@", - [NSError _last]]; - [readInfo setObject: s forKey: GSFileHandleNotificationError]; - [self postReadNotification]; - } - } - else - { - [item appendBytes: buf length: received]; - if (readMax < 0 || (readMax > 0 && (int)[item length] == readMax)) + else if (received < 0) { - // Read a single chunk of data - [self postReadNotification]; + if (isSocket && (WSAGetLastError() != WSAEINTR + && WSAGetLastError() != WSAEWOULDBLOCK)) + { + NSString *s; + + s = [NSString stringWithFormat: @"Read attempt failed - %@", + [NSError _last]]; + [readInfo setObject: s forKey: GSFileHandleNotificationError]; + [self postReadNotification]; + } + else if (!isSocket && (GetLastError() != ERROR_NO_DATA)) + { + NSString *s; + + s = [NSString stringWithFormat: @"Read attempt failed - %@", + [NSError _last]]; + [readInfo setObject: s forKey: GSFileHandleNotificationError]; + [self postReadNotification]; + } + } + else + { + [item appendBytes: buf length: received]; + if (readMax < 0 || (readMax > 0 && (int)[item length] == readMax)) + { + // Read a single chunk of data + [self postReadNotification]; + } } } } diff --git a/Source/win32/GSRunLoopCtxt.m b/Source/win32/GSRunLoopCtxt.m index 2fcffb5b1..f52a3d5d8 100644 --- a/Source/win32/GSRunLoopCtxt.m +++ b/Source/win32/GSRunLoopCtxt.m @@ -486,6 +486,7 @@ static const NSMapTableValueCallBacks WatcherMapValueCallBacks = watcher = (GSRunLoopWatcher*)GSIArrayItemAtIndex(_trigger, count).obj; if (watcher->_invalidated == NO) { + NSDebugMLLog(@"NSRunLoop", @"trigger watcher %@", watcher); i = [contexts count]; while (i-- > 0) { @@ -511,6 +512,7 @@ static const NSMapTableValueCallBacks WatcherMapValueCallBacks = // if there are windows message if (wait_return == WAIT_OBJECT_0 + num_handles) { + NSDebugMLLog(@"NSRunLoop", @"processing windows messages"); [self processAllWindowsMessages: num_winMsgs within: contexts]; return NO; } @@ -518,6 +520,7 @@ static const NSMapTableValueCallBacks WatcherMapValueCallBacks = // if there aren't events if (wait_return == WAIT_TIMEOUT) { + NSDebugMLLog(@"NSRunLoop", @"timeout without events"); completed = YES; return NO; } diff --git a/Source/win32/NSMessagePort.m b/Source/win32/NSMessagePort.m index 69231d94f..a8de60613 100644 --- a/Source/win32/NSMessagePort.m +++ b/Source/win32/NSMessagePort.m @@ -194,7 +194,7 @@ static Class messagePortClass = 0; security.nLength = sizeof(SECURITY_ATTRIBUTES); security.lpSecurityDescriptor = 0; // Default - security.bInheritHandle = TRUE; + security.bInheritHandle = FALSE; } } diff --git a/Source/win32/NSMessagePortNameServer.m b/Source/win32/NSMessagePortNameServer.m index 0b0ee5f9c..fb189ac69 100644 --- a/Source/win32/NSMessagePortNameServer.m +++ b/Source/win32/NSMessagePortNameServer.m @@ -105,7 +105,7 @@ static void clean_up_names(void) security.nLength = sizeof(SECURITY_ATTRIBUTES); security.lpSecurityDescriptor = 0; // Default - security.bInheritHandle = TRUE; + security.bInheritHandle = FALSE; registry = @"Software\\GNUstepNSMessagePort"; rc = RegCreateKeyExW( diff --git a/Source/win32/NSStream.m b/Source/win32/NSStream.m index f4499594f..e81c24910 100644 --- a/Source/win32/NSStream.m +++ b/Source/win32/NSStream.m @@ -1016,7 +1016,7 @@ fileSystemRepresentation]; saAttr.nLength = sizeof(SECURITY_ATTRIBUTES); - saAttr.bInheritHandle = TRUE; + saAttr.bInheritHandle = FALSE; saAttr.lpSecurityDescriptor = NULL; handle = CreateFileW(name, @@ -1067,7 +1067,7 @@ done: int rc; saAttr.nLength = sizeof(SECURITY_ATTRIBUTES); - saAttr.bInheritHandle = TRUE; + saAttr.bInheritHandle = FALSE; saAttr.lpSecurityDescriptor = NULL; /* @@ -1346,7 +1346,7 @@ done: NSAssert(handle == INVALID_HANDLE_VALUE, NSInternalInconsistencyException); saAttr.nLength = sizeof(SECURITY_ATTRIBUTES); - saAttr.bInheritHandle = TRUE; + saAttr.bInheritHandle = FALSE; saAttr.lpSecurityDescriptor = NULL; handle = CreateNamedPipeW([path fileSystemRepresentation],