fixups for win32 pipe management

git-svn-id: svn+ssh://svn.gna.org/svn/gnustep/libs/base/trunk@28747 72102866-910b-0410-8b05-ffd578937521

ChangeLog

@@ -1,3 +1,22 @@
+2009-09-27 Richard Frith-Macdonald <rfm@gnu.org>
+
+	* 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 <rfm@gnu.org>
 
 	* Source/NSURL.m: OSX compatibility tweaks ... allows initialisation

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)

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 (_standardInput == nil)
-  if ([hdl isKindOfClass: [NSPipe class]])
     {
-      hdl = [(NSPipe*)hdl fileHandleForWriting];
+      start_info.hStdInput = GetStdHandle(STD_INPUT_HANDLE);
-      [toClose addObject: hdl];
     }
-  start_info.hStdOutput = [hdl nativeHandle];
+  else
-
-  hdl = [self standardError];
-  if ([hdl isKindOfClass: [NSPipe class]])
     {
-      hdl = [(NSPipe*)hdl fileHandleForWriting];
+      hdl = [self standardInput];
-      /*
+      if ([hdl isKindOfClass: [NSPipe class]])
-       * If we have the same pipe twice we don't want to close it twice
-       */
-      if ([toClose indexOfObjectIdenticalTo: hdl] == NSNotFound)
 	{
+	  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,
+    0
-    CREATE_NO_WINDOW|CREATE_UNICODE_ENVIRONMENT,
+//    |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;
     }
 

Source/win32/GSFileHandle.m

@@ -2003,68 +2003,99 @@ NSString * const GSSOCKSRecvAddr = @"GSSOCKSRecvAddr";
 	}
       [self postReadNotification];
     }
-  else if (operation == NSFileHandleDataAvailableNotification)
-    {
-      [self postReadNotification];
-    }
   else
     {
-      NSMutableData	*item;
+      /* If this is not a socket or a standard file, we assume it's a pipe
-      int		length;
+       * and therefore we need to check to see if data really is available.
-      int		received = 0;
-      char		buf[READ_SIZE];
-
-      item = [readInfo objectForKey: NSFileHandleNotificationDataItem];
-      /*
-       * We may have a maximum data size set...
        */
-      if (readMax > 0)
+      if (NO == isSocket && NO == isStandardFile)
-        {
+	{
-          length = (unsigned int)readMax - [item length];
+	  HANDLE	h = (HANDLE)_get_osfhandle(descriptor);
-          if (length > (int)sizeof(buf))
+	  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];
+	  received = [self read: buf length: length];
-      if (received == 0)
+	  if (received == 0)
-        { // Read up to end of file.
+	    { // 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];
 	      [self postReadNotification];
 	    }
-          else if (!isSocket && (GetLastError() != ERROR_NO_DATA))
+	  else if (received < 0)
-            {
-	      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
+	      if (isSocket && (WSAGetLastError() != WSAEINTR
-	      [self postReadNotification];
+		&& 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];
+		}
 	    }
 	}
     }

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;        
     }

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;
     }
 }
 

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(

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],