This is a libiberty patch for Windows that we've had in our local tree for a 
couple of years now.  The problem it addresses showed up in Windows-hosted GDB; 
we were getting a crash inside _close() in the case where creation of the child 
process has failed. 

Apparently Windows gets very unhappy if you try to close an already-closed file 
descriptor a second time.  This documentation, which turned up at the top of a 
Google search,

http://msdn.microsoft.com/en-us/library/5fzwd5ss%28v=vs.80%29.aspx
http://msdn.microsoft.com/en-us/library/ksazx244%28v=vs.80%29.aspx

seems to indicate that crashing is the default behavior if "fd is a bad file 
descriptor".

The duplicate calls causing the crash in pex_run_in_environment are pretty easy 
to avoid.  In the unmodified code, the stdin/stdout/stderr file descriptors are 
closed once in pex_win32_exec_child, immediately after they are dup'ed for the 
child; if process creation fails, they get closed a second time in 
pex_run_in_environment.  This patch fixes pex_win32_exec_child to only close 
the original file descriptors if process creation succeeds, leaving 
pex_run_in_environment to take care of the failure case.

We haven't tried to monitor other uses of _close in libiberty.  I was looking 
around for a Windows function to test whether a file descriptor is already 
closed that we could guard the calls in pex_win32_close with, but didn't see 
anything.  In any case, this patch has been working well enough for us, as far 
as it goes, and is at least an incremental improvement in robustness.  OK for 
mainline?

-Sandra


2012-07-26  Kazu Hirata  <k...@codesourcery.com>
            Sandra Loosemore  <san...@codesourcery.com>

        libiberty/
        * pex-win32.c (pex_win32_exec_child): Only close original file
        descriptors if child is launched successfully.

Index: libiberty/pex-win32.c
===================================================================
--- libiberty/pex-win32.c	(revision 189895)
+++ libiberty/pex-win32.c	(working copy)
@@ -741,24 +741,17 @@ pex_win32_exec_child (struct pex_obj *ob
   int orig_out, orig_in, orig_err;
   BOOL separate_stderr = !(flags & PEX_STDERR_TO_STDOUT);
 
-  /* Ensure we have inheritable descriptors to pass to the child, and close the
-     original descriptors.  */
+  /* Ensure we have inheritable descriptors to pass to the child.  */
   orig_in = in;
   in = _dup (orig_in);
-  if (orig_in != STDIN_FILENO)
-    _close (orig_in);
   
   orig_out = out;
   out = _dup (orig_out);
-  if (orig_out != STDOUT_FILENO)
-    _close (orig_out);
   
   if (separate_stderr)
     {
       orig_err = errdes;
       errdes = _dup (orig_err);
-      if (orig_err != STDERR_FILENO)
-	_close (orig_err);
     }
 
   stdin_handle = INVALID_HANDLE_VALUE;
@@ -836,6 +829,22 @@ pex_win32_exec_child (struct pex_obj *ob
       *errmsg = "CreateProcess";
     }
 
+  /* If the child was created successfully, close the original file
+     descriptors.  If the process creation fails, these are closed by
+     pex_run_in_environment instead.  We must not close them twice as
+     that seems to cause a Windows exception.  */
+     
+  if (pid != (pid_t) -1)
+    {
+      if (orig_in != STDIN_FILENO)
+	_close (orig_in);
+      if (orig_out != STDOUT_FILENO)
+	_close (orig_out);
+      if (separate_stderr
+	  && orig_err != STDERR_FILENO)
+	_close (orig_err);
+    }
+
   /* Close the standard input, standard output and standard error handles
      in the parent.  */ 
 

Reply via email to