Fix [c54e4a1aeb]: High Tcl latencies with fork() in larger systems

6 files changed, 171 insertions, 29 deletions

diff --git a/unix/configure b/unix/configure
index 54e3ae6..d1e93c2 100755
--- a/unix/configure
+++ b/unix/configure
@@ -8978,6 +8978,109 @@ echo "${ECHO_T}$ac_cv_nolto" >&6
 	CFLAGS_NOLTO=""
     fi
 
+
+for ac_func in posix_spawnp
+do
+as_ac_var=`echo "ac_cv_func_$ac_func" | $as_tr_sh`
+echo "$as_me:$LINENO: checking for $ac_func" >&5
+echo $ECHO_N "checking for $ac_func... $ECHO_C" >&6
+if eval "test \"\${$as_ac_var+set}\" = set"; then
+  echo $ECHO_N "(cached) $ECHO_C" >&6
+else
+  cat >conftest.$ac_ext <<_ACEOF
+/* confdefs.h.  */
+_ACEOF
+cat confdefs.h >>conftest.$ac_ext
+cat >>conftest.$ac_ext <<_ACEOF
+/* end confdefs.h.  */
+/* Define $ac_func to an innocuous variant, in case <limits.h> declares $ac_func.
+   For example, HP-UX 11i <limits.h> declares gettimeofday.  */
+#define $ac_func innocuous_$ac_func
+
+/* System header to define __stub macros and hopefully few prototypes,
+    which can conflict with char $ac_func (); below.
+    Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
+    <limits.h> exists even on freestanding compilers.  */
+
+#ifdef __STDC__
+# include <limits.h>
+#else
+# include <assert.h>
+#endif
+
+#undef $ac_func
+
+/* Override any gcc2 internal prototype to avoid an error.  */
+#ifdef __cplusplus
+extern "C"
+{
+#endif
+/* We use char because int might match the return type of a gcc2
+   builtin and then its argument prototype would still apply.  */
+char $ac_func ();
+/* The GNU C library defines this for functions which it implements
+    to always fail with ENOSYS.  Some functions are actually named
+    something starting with __ and the normal name is an alias.  */
+#if defined (__stub_$ac_func) || defined (__stub___$ac_func)
+choke me
+#else
+char (*f) () = $ac_func;
+#endif
+#ifdef __cplusplus
+}
+#endif
+
+int
+main ()
+{
+return f != $ac_func;
+  ;
+  return 0;
+}
+_ACEOF
+rm -f conftest.$ac_objext conftest$ac_exeext
+if { (eval echo "$as_me:$LINENO: \"$ac_link\"") >&5
+  (eval $ac_link) 2>conftest.er1
+  ac_status=$?
+  grep -v '^ *+' conftest.er1 >conftest.err
+  rm -f conftest.er1
+  cat conftest.err >&5
+  echo "$as_me:$LINENO: \$? = $ac_status" >&5
+  (exit $ac_status); } &&
+	 { ac_try='test -z "$ac_c_werror_flag"
+			 || test ! -s conftest.err'
+  { (eval echo "$as_me:$LINENO: \"$ac_try\"") >&5
+  (eval $ac_try) 2>&5
+  ac_status=$?
+  echo "$as_me:$LINENO: \$? = $ac_status" >&5
+  (exit $ac_status); }; } &&
+	 { ac_try='test -s conftest$ac_exeext'
+  { (eval echo "$as_me:$LINENO: \"$ac_try\"") >&5
+  (eval $ac_try) 2>&5
+  ac_status=$?
+  echo "$as_me:$LINENO: \$? = $ac_status" >&5
+  (exit $ac_status); }; }; then
+  eval "$as_ac_var=yes"
+else
+  echo "$as_me: failed program was:" >&5
+sed 's/^/| /' conftest.$ac_ext >&5
+
+eval "$as_ac_var=no"
+fi
+rm -f conftest.err conftest.$ac_objext \
+      conftest$ac_exeext conftest.$ac_ext
+fi
+echo "$as_me:$LINENO: result: `eval echo '${'$as_ac_var'}'`" >&5
+echo "${ECHO_T}`eval echo '${'$as_ac_var'}'`" >&6
+if test `eval echo '${'$as_ac_var'}'` = yes; then
+  cat >>confdefs.h <<_ACEOF
+#define `echo "HAVE_$ac_func" | $as_tr_cpp` 1
+_ACEOF
+
+fi
+done
+
+
     # FIXME: This subst was left in only because the TCL_DL_LIBS
     # entry in tclConfig.sh uses it. It is not clear why someone
     # would use TCL_DL_LIBS instead of TCL_LIBS.
@@ -17842,11 +17945,6 @@ done
     fi
 
 cat >>confdefs.h <<\_ACEOF
-#define USE_VFORK 1
-_ACEOF
-
-
-cat >>confdefs.h <<\_ACEOF
 #define TCL_DEFAULT_ENCODING "utf-8"
 _ACEOF
 
diff --git a/unix/configure.in b/unix/configure.in
index 14922a6..3e80626 100644
--- a/unix/configure.in
+++ b/unix/configure.in
@@ -563,7 +563,6 @@ if test "`uname -s`" = "Darwin" ; then
 	AC_CHECK_HEADERS(libkern/OSAtomic.h)
 	AC_CHECK_FUNCS(OSSpinLockLock)
     fi
-    AC_DEFINE(USE_VFORK, 1, [Should we use vfork() instead of fork()?])
     AC_DEFINE(TCL_DEFAULT_ENCODING, "utf-8",
 	[Are we to override what our default encoding is?])
     AC_DEFINE(TCL_LOAD_FROM_MEMORY, 1,
diff --git a/unix/tcl.m4 b/unix/tcl.m4
index 0ef9f3d..add827f 100644
--- a/unix/tcl.m4
+++ b/unix/tcl.m4
@@ -2068,6 +2068,8 @@ dnl # preprocessing tests use only CPPFLAGS.
 	CFLAGS_NOLTO=""
     fi
 
+    AC_CHECK_FUNCS([posix_spawnp])
+
     # FIXME: This subst was left in only because the TCL_DL_LIBS
     # entry in tclConfig.sh uses it. It is not clear why someone
     # would use TCL_DL_LIBS instead of TCL_LIBS.
@@ -2478,12 +2480,12 @@ AC_DEFUN([SC_TCL_LINK_LIBS], [
 AC_DEFUN([SC_TCL_EARLY_FLAG],[
     AC_CACHE_VAL([tcl_cv_flag_]translit($1,[A-Z],[a-z]),
 	AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[$2]], [[$3]])],
-	    [tcl_cv_flag_]translit($1,[A-Z],[a-z])=no,[AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[[#define ]$1[ 1
+	    [tcl_cv_flag_]translit($1,[A-Z],[a-z])=no,[AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[[#define ]$1[ ]m4_default([$4],[1])[
 ]$2]], [[$3]])],
 	[tcl_cv_flag_]translit($1,[A-Z],[a-z])=yes,
 	[tcl_cv_flag_]translit($1,[A-Z],[a-z])=no)]))
     if test ["x${tcl_cv_flag_]translit($1,[A-Z],[a-z])[}" = "xyes"] ; then
-	AC_DEFINE($1, 1, [Add the ]$1[ flag when building])
+	AC_DEFINE($1, m4_default([$4],[1]), [Add the ]$1[ flag when building])
 	tcl_flags="$tcl_flags $1"
     fi
 ])
diff --git a/unix/tclConfig.h.in b/unix/tclConfig.h.in
index 6d559d1..486d8d9 100644
--- a/unix/tclConfig.h.in
+++ b/unix/tclConfig.h.in
@@ -174,6 +174,9 @@
 /* Define to 1 if you have the `OSSpinLockLock' function. */
 #undef HAVE_OSSPINLOCKLOCK
 
+/* Define to 1 if you have the `posix_spawnp' function. */
+#undef HAVE_POSIX_SPAWNP
+
 /* Define to 1 if you have the `pthread_atfork' function. */
 #undef HAVE_PTHREAD_ATFORK
 
@@ -447,9 +450,6 @@
 /* Do we want to use the threaded memory allocator? */
 #undef USE_THREAD_ALLOC
 
-/* Should we use vfork() instead of fork()? */
-#undef USE_VFORK
-
 /* Define to 1 if your processor stores words with the most significant byte
    first (like Motorola and SPARC, unlike Intel and VAX). */
 #undef WORDS_BIGENDIAN
diff --git a/unix/tclUnixPipe.c b/unix/tclUnixPipe.c
index 75e2879..649fa4d 100644
--- a/unix/tclUnixPipe.c
+++ b/unix/tclUnixPipe.c
@@ -13,6 +13,10 @@
 
 #include "tclInt.h"
 
+#ifdef HAVE_POSIX_SPAWNP
+# include <spawn.h>
+#endif
+
 #ifdef USE_VFORK
 #define fork vfork
 #endif
@@ -411,6 +415,9 @@ TclpCreateProcess(
     Tcl_DString *dsArray;
     char **newArgv;
     int pid, i;
+#if defined(HAVE_POSIX_SPAWNP)
+    int childErrno;
+#endif
 
     errPipeIn = NULL;
     errPipeOut = NULL;
@@ -439,7 +446,7 @@ TclpCreateProcess(
 	newArgv[i] = Tcl_UtfToExternalDString(NULL, argv[i], -1, &dsArray[i]);
     }
 
-#ifdef USE_VFORK
+#if defined(USE_VFORK) || defined(HAVE_POSIX_SPAWNP)
     /*
      * After vfork(), do not call code in the child that changes global state,
      * because it is using the parent's memory space at that point and writes
@@ -452,14 +459,54 @@ TclpCreateProcess(
 	Tcl_GetStdChannel(TCL_STDIN);
     }
     if (!outputFile) {
-        Tcl_GetStdChannel(TCL_STDOUT);
+	Tcl_GetStdChannel(TCL_STDOUT);
     }
     if (!errorFile) {
-        Tcl_GetStdChannel(TCL_STDERR);
+	Tcl_GetStdChannel(TCL_STDERR);
     }
 #endif
 
+#ifdef HAVE_POSIX_SPAWNP
+    {
+	posix_spawn_file_actions_t actions;
+	posix_spawnattr_t attr;
+
+	posix_spawn_file_actions_init(&actions);
+	posix_spawnattr_init(&attr);
+
+	posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETSIGDEF|
+# ifdef POSIX_SPAWN_USEVFORK
+		POSIX_SPAWN_USEVFORK
+# else
+		0
+# endif
+		);
+
+	posix_spawn_file_actions_adddup2(&actions, GetFd(inputFile), 0);
+	posix_spawn_file_actions_adddup2(&actions, GetFd(outputFile), 1);
+	posix_spawn_file_actions_adddup2(&actions, GetFd(errorFile), 2);
+
+	status = posix_spawnp(&pid, newArgv[0], &actions, &attr, newArgv, environ);
+	childErrno = status;
+
+	posix_spawn_file_actions_destroy(&actions);
+	posix_spawnattr_destroy(&attr);
+
+	/*
+	 * Fork semantics:
+	 *  - pid == 0: child process
+	 *  - pid == -1: error
+	 *  - pid > 0: parent process
+	 *
+	 * Mimic fork semantics to minimize changes below
+	 */
+	if (status != 0) {
+	    pid = -1;
+	}
+    }
+#else
     pid = fork();
+#endif
     if (pid == 0) {
 	size_t len;
 	int joinThisError = errorFile && (errorFile == outputFile);
@@ -509,8 +556,14 @@ TclpCreateProcess(
     TclStackFree(interp, dsArray);
 
     if (pid == -1) {
+#ifdef HAVE_POSIX_SPAWNP
+	errno = childErrno;
+	Tcl_SetObjResult(interp, Tcl_ObjPrintf(
+		"couldn't execute \"%s\": %s", argv[0], Tcl_PosixError(interp)));
+#else
 	Tcl_SetObjResult(interp, Tcl_ObjPrintf(
 		"couldn't fork child process: %s", Tcl_PosixError(interp)));
+#endif
 	goto error;
     }
 
diff --git a/unix/tclUnixPort.h b/unix/tclUnixPort.h
index 97caad0..cf0e548 100644
--- a/unix/tclUnixPort.h
+++ b/unix/tclUnixPort.h
@@ -636,23 +636,13 @@ extern char **		environ;
 	    defined(HAVE_WEAK_IMPORT) && MAC_OS_X_VERSION_MIN_REQUIRED < 1050
 #	warning "Weak import of 64-bit CoreFoundation is not supported, will not run on Mac OS X < 10.5."
 #   endif
-
-/*
- *---------------------------------------------------------------------------
- * At present, using vfork() instead of fork() causes execve() to fail
- * intermittently on Darwin x86_64. rdar://4685553
- *---------------------------------------------------------------------------
+/* For now, test exec-17.1 fails (I/O setup after closing stdout) with
+ * posix_spawnp(), but the classic implementation (based on fork()+execvp())
+ * works well under macOS quite OK.
  */
-
-#   if defined(__x86_64__) && !defined(FIXED_RDAR_4685553)
-#	undef USE_VFORK
+#   if defined(HAVE_POSIX_SPAWNP)
+#	undef HAVE_POSIX_SPAWNP
 #   endif /* __x86_64__ */
-/* Workaround problems with vfork() when building with llvm-gcc-4.2 */
-#   if defined (__llvm__) && \
-	    (__GNUC__ > 4 || (__GNUC__ == 4 && (__GNUC_MINOR__ > 2 || \
-	    (__GNUC_MINOR__ == 2 && __GNUC_PATCHLEVEL__ > 0))))
-#	undef USE_VFORK
-#   endif /* __llvm__ */
 #endif /* __APPLE__ */
 
 /*