Patchwork Use cygwin spawn functions

login
register
mail settings
Submitter Richard Henderson
Date Oct. 12, 2010, 12:41 a.m.
Message ID <4CB3AEC5.3090209@redhat.com>
Download mbox | patch
Permalink /patch/67497/
State New
Headers show

Comments

Richard Henderson - Oct. 12, 2010, 12:41 a.m.
Emulating fork(2) on cygwin is a bit of a challenge; the hoop jumping is
a regular circus act.  Worse, fork has a habit of failing on Windows 7 64-bit,
as seen in the many "Bad address" and "Resource temporarily unavailable"
errors during testing
(e.g. http://gcc.gnu.org/ml/gcc-testresults/2010-10/msg00624.html).

Cygwin's own faq (sec 5.5) says that it's better to avoid using fork+exec
when possible and use the spawn family of calls instead.

With this, I managed an entire check-gcc run without a single spurious
fork failure for --host=i686-cygwin --target=x86_64-w64-mingw64.

Like so.  Ok to commit?


r~
* configure.ac (process.h): Test for it.
	(dup3, spawnvpe): Test for them.
	* configure, config.in: Rebuild.
	* pex-unix.c: Include process.h if available.
	(struct save_fd, save_and_install_fd, restore_fd): New.
	(pex_unix_exec_child): New implementation using spawnvpe.
Dave Korn - Oct. 12, 2010, 1:20 a.m.
On 12/10/2010 01:41, Richard Henderson wrote:
> Emulating fork(2) on cygwin is a bit of a challenge; the hoop jumping is
> a regular circus act.  Worse, fork has a habit of failing on Windows 7 64-bit,
> as seen in the many "Bad address" and "Resource temporarily unavailable"
> errors during testing
> (e.g. http://gcc.gnu.org/ml/gcc-testresults/2010-10/msg00624.html).
> 
> Cygwin's own faq (sec 5.5) says that it's better to avoid using fork+exec
> when possible and use the spawn family of calls instead.

  It's a ton less overhead too, hopefully this will show up in faster test
runs; thanks for that.

    cheers,
      DaveK
NightStrike - Oct. 12, 2010, 2:20 p.m.
On Mon, Oct 11, 2010 at 8:41 PM, Richard Henderson <rth@redhat.com> wrote:
> Emulating fork(2) on cygwin is a bit of a challenge; the hoop jumping is
> a regular circus act.  Worse, fork has a habit of failing on Windows 7 64-bit,
> as seen in the many "Bad address" and "Resource temporarily unavailable"
> errors during testing
> (e.g. http://gcc.gnu.org/ml/gcc-testresults/2010-10/msg00624.html).
>
> Cygwin's own faq (sec 5.5) says that it's better to avoid using fork+exec
> when possible and use the spawn family of calls instead.
>
> With this, I managed an entire check-gcc run without a single spurious
> fork failure for --host=i686-cygwin --target=x86_64-w64-mingw64.
>
> Like so.  Ok to commit?

THANK YOU!!!!!!!!!11oneone!!!!!!!!!!!
Richard Henderson - Oct. 12, 2010, 3:44 p.m.
On 10/11/2010 05:41 PM, Richard Henderson wrote:
> Emulating fork(2) on cygwin is a bit of a challenge; the hoop jumping is
> a regular circus act.  Worse, fork has a habit of failing on Windows 7 64-bit,
> as seen in the many "Bad address" and "Resource temporarily unavailable"
> errors during testing
> (e.g. http://gcc.gnu.org/ml/gcc-testresults/2010-10/msg00624.html).
> 
> Cygwin's own faq (sec 5.5) says that it's better to avoid using fork+exec
> when possible and use the spawn family of calls instead.
> 
> With this, I managed an entire check-gcc run without a single spurious
> fork failure for --host=i686-cygwin --target=x86_64-w64-mingw64.
> 
> Like so.  Ok to commit?

Hum.  Actually, something is Not Quite Right with it.

On both WinXP-32 and Win7-64 hosts, a full test run eventually got to a
point where every test would time out.  In both cases, collect2 appears
to have gotten stuck.  More investigation is required.


r~

Patch

diff --git a/libiberty/configure.ac b/libiberty/configure.ac
index 73ea6c9..34a1a7e 100644
--- a/libiberty/configure.ac
+++ b/libiberty/configure.ac
@@ -246,7 +246,7 @@  AC_SUBST_FILE(host_makefile_frag)
 # It's OK to check for header files.  Although the compiler may not be
 # able to link anything, it had better be able to at least compile
 # something.
-AC_CHECK_HEADERS(sys/file.h sys/param.h limits.h stdlib.h malloc.h string.h unistd.h strings.h sys/time.h time.h sys/resource.h sys/stat.h sys/mman.h fcntl.h alloca.h sys/pstat.h sys/sysmp.h sys/sysinfo.h machine/hal_sysinfo.h sys/table.h sys/sysctl.h sys/systemcfg.h stdint.h stdio_ext.h)
+AC_CHECK_HEADERS(sys/file.h sys/param.h limits.h stdlib.h malloc.h string.h unistd.h strings.h sys/time.h time.h sys/resource.h sys/stat.h sys/mman.h fcntl.h alloca.h sys/pstat.h sys/sysmp.h sys/sysinfo.h machine/hal_sysinfo.h sys/table.h sys/sysctl.h sys/systemcfg.h stdint.h stdio_ext.h process.h)
 AC_HEADER_SYS_WAIT
 AC_HEADER_TIME
 
@@ -359,14 +359,14 @@  vars="sys_errlist sys_nerr sys_siglist"
 
 checkfuncs="getrusage on_exit psignal strerror strsignal sysconf times sbrk gettimeofday"
 checkfuncs="$checkfuncs realpath canonicalize_file_name pstat_getstatic pstat_getdynamic sysmp"
-checkfuncs="$checkfuncs getsysinfo table sysctl wait3 wait4 __fsetlocking"
+checkfuncs="$checkfuncs getsysinfo table sysctl wait3 wait4 __fsetlocking dup3 spawnvpe"
 
 # These are neither executed nor required, but they help keep
 # autoheader happy without adding a bunch of text to acconfig.h.
 if test "x" = "y"; then
   AC_CHECK_FUNCS(asprintf atexit \
     basename bcmp bcopy bsearch bzero \
-    calloc canonicalize_file_name clock \
+    calloc canonicalize_file_name clock dup3 \
     ffs __fsetlocking \
     getcwd getpagesize getrusage getsysinfo gettimeofday \
     index insque \
@@ -374,10 +374,10 @@  if test "x" = "y"; then
     on_exit \
     psignal pstat_getdynamic pstat_getstatic putenv \
     random realpath rename rindex \
-    sbrk setenv setproctitle sigsetmask snprintf stpcpy stpncpy strcasecmp strchr \
-    strdup \
+    sbrk setenv setproctitle sigsetmask snprintf stpcpy stpncpy \
+     strcasecmp strchr strdup \
      strerror strncasecmp strndup strrchr strsignal strstr strtod strtol \
-     strtoul strverscmp sysconf sysctl sysmp \
+     strtoul strverscmp sysconf sysctl sysmp spawnvpe \
     table times tmpnam \
     vasprintf vfprintf vprintf vsprintf \
     wait3 wait4 waitpid)
diff --git a/libiberty/pex-unix.c b/libiberty/pex-unix.c
index 85733a6..eaf9c3e 100644
--- a/libiberty/pex-unix.c
+++ b/libiberty/pex-unix.c
@@ -55,7 +55,9 @@  extern int errno;
 #ifdef HAVE_SYS_STAT_H
 #include <sys/stat.h>
 #endif
-
+#ifdef HAVE_PROCESS_H
+#include <process.h>
+#endif
 
 #ifdef vfork /* Autoconf may define this to fork for us. */
 # define VFORK_STRING "fork"
@@ -387,6 +389,106 @@  pex_child_error (struct pex_obj *obj, const char *executable,
 
 extern char **environ;
 
+#ifdef HAVE_SPAWNVPE
+/* Helpers for saving file descriptors around spawning a child.  */
+
+struct save_fd
+{
+  int fd;
+  int flags;
+};
+
+static int
+save_and_install_fd(struct save_fd *s, int old_fd, int child_fd)
+{
+  s->fd = -1;
+  s->flags = 0;
+
+  if (old_fd == child_fd)
+    return;
+
+  s->fd = dup (old_fd);
+  if (s->fd >= 0)
+    {
+      s->flags = fcntl (old_fd, F_GETFD);
+      fcntl (s->fd, F_SETFD, FD_CLOEXEC);
+    }
+  else
+    return -1;
+
+  fcntl (child_fd, F_SETFD, FD_CLOEXEC);
+  return dup2 (child_fd, old_fd);
+}
+
+static void
+restore_fd(struct save_fd *s, int old_fd)
+{
+  if (s->fd < 0)
+    return;
+#ifdef HAVE_DUP3
+  else if (s->flags == FD_CLOEXEC)
+    dup3 (s->fd, old_fd, O_CLOEXEC);
+#endif
+  else
+    {
+      dup2 (s->fd, old_fd);
+      if (s->flags != 0)
+	fcntl (old_fd, F_SETFD, s->flags);
+    }
+  close (s->fd);
+}
+
+static pid_t
+pex_unix_exec_child (struct pex_obj *obj, int flags, const char *executable,
+		     char * const * argv, char * const * env,
+                     int in, int out, int errdes,
+		     int toclose, const char **errmsg, int *err)
+{
+  struct save_fd save_in, save_out, save_err;
+  int sleep_interval, retries;
+  pid_t pid;
+
+  if (save_and_install_fd (&save_in, STDIN_FILE_NO, in) < 0
+      || save_and_install_fd (&save_out, STDOUT_FILE_NO, out) < 0
+      || save_and_install_fd (&save_err, STDERR_FILE_NO, errdes) < 0)
+    {
+      *err = errno;
+      *errmsg = "dup2";
+      return -1;
+    }
+
+  if (env == NULL)
+    env = environ;
+
+  sleep_interval = 1;
+  pid = -1;
+  for (retries = 0; retries < 4; ++retries)
+    {
+      if (flags & PEX_SEARCH)
+	pid = spawnvpe (_P_NOWAITO, executable, argv, env);
+      else
+	pid = spawnve (_P_NOWAITO, executable, argv, env);
+
+      if (pid > 0)
+	{
+	  /* Success.  */
+	  restore_fd (&save_in, STDIN_FILE_NO);
+	  restore_fd (&save_out, STDOUT_FILE_NO);
+	  restore_fd (&save_err, STDERR_FILE_NO);
+	  return pid;
+	}
+
+      if (errno != EAGAIN)
+	break;
+      sleep (sleep_interval);
+      sleep_interval *= 2;
+    }
+
+  *err = errno;
+  *errmsg = "spawn";
+  return -1;
+}
+#else
 static pid_t
 pex_unix_exec_child (struct pex_obj *obj, int flags, const char *executable,
 		     char * const * argv, char * const * env,
@@ -521,6 +623,7 @@  pex_unix_exec_child (struct pex_obj *obj, int flags, const char *executable,
       return pid;
     }
 }
+#endif /* SPAWN */
 
 /* Wait for a child process to complete.  */