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 <[email protected]>
Sandra Loosemore <[email protected]>
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. */
