emacs-diffs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

scratch/posix-spawn-no-gnulib c28ccdf: Use posix_spawn if possible.


From: Philipp Stephani
Subject: scratch/posix-spawn-no-gnulib c28ccdf: Use posix_spawn if possible.
Date: Thu, 31 Dec 2020 10:18:06 -0500 (EST)

branch: scratch/posix-spawn-no-gnulib
commit c28ccdff224ea5c1c7d43222fbc7df2b3e6336b5
Author: Philipp Stephani <phst@google.com>
Commit: Philipp Stephani <phst@google.com>

    Use posix_spawn if possible.
    
    posix_spawn is less error-prone than vfork + execve, and can make
    better use of system-specific enhancements like 'clone' on Linux.  Use
    it if we don't need to configure a pseudoterminal.
    
    * configure.ac (HAVE_SPAWN_H, HAVE_POSIX_SPAWN)
    (HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR)
    (HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP)
    (HAVE_POSIX_SPAWNATTR_SETFLAGS, HAVE_DECL_POSIX_SPAWN_SETSID): New
    configuration variables.
    * src/callproc.c (USABLE_POSIX_SPAWN): New configuration macro.
    (emacs_posix_spawn_init_actions)
    (emacs_posix_spawn_init_attributes, emacs_posix_spawn_init): New
    helper functions.
    (emacs_spawn): Use posix_spawn if possible.
---
 configure.ac   |  17 ++++++
 src/callproc.c | 189 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 205 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index bf76844..7793fa5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -4539,6 +4539,23 @@ dnl AC_CHECK_FUNCS_ONCE wouldn’t be right for snprintf, 
which needs
 dnl the current CFLAGS etc.
 AC_CHECK_FUNCS(snprintf)
 
+dnl posix_spawn.  The chdir and setsid functionality is relatively
+dnl recent, so we check for it specifically.
+AC_CHECK_HEADERS([spawn.h])
+AC_SUBST([HAVE_SPAWN_H])
+AC_CHECK_FUNCS([posix_spawn \
+                posix_spawn_file_actions_addchdir \
+                posix_spawn_file_actions_addchdir_np \
+                posix_spawnattr_setflags])
+AC_SUBST([HAVE_POSIX_SPAWN])
+AC_SUBST([HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR])
+AC_SUBST([HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP])
+AC_SUBST([HAVE_POSIX_SPAWNATTR_SETFLAGS])
+AC_CHECK_DECLS([POSIX_SPAWN_SETSID], [], [], [[
+               #include <spawn.h>
+               ]])
+AC_SUBST([HAVE_DECL_POSIX_SPAWN_SETSID])
+
 dnl Check for glib.  This differs from other library checks in that
 dnl Emacs need not link to glib unless some other library is already
 dnl linking to glib.  Although glib provides no facilities that Emacs
diff --git a/src/callproc.c b/src/callproc.c
index 3ecd688..cb270d4 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -28,6 +28,20 @@ along with GNU Emacs.  If not, see 
<https://www.gnu.org/licenses/>.  */
 #include <sys/file.h>
 #include <fcntl.h>
 
+/* In order to be able to use `posix_spawn', it needs to support some
+   variant of `chdir' as well as `setsid'.  */
+#if defined HAVE_SPAWN_H && defined HAVE_POSIX_SPAWN        \
+  && defined HAVE_POSIX_SPAWNATTR_SETFLAGS                  \
+  && (defined HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR        \
+      || defined HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP) \
+  && defined HAVE_DECL_POSIX_SPAWN_SETSID                   \
+  && HAVE_DECL_POSIX_SPAWN_SETSID == 1
+# include <spawn.h>
+# define USABLE_POSIX_SPAWN 1
+#else
+# define USABLE_POSIX_SPAWN 0
+#endif
+
 #include "lisp.h"
 
 #ifdef WINDOWSNT
@@ -1215,6 +1229,130 @@ child_setup (int in, int out, int err, char **new_argv, 
char **env,
 #endif  /* not WINDOWSNT */
 }
 
+#if USABLE_POSIX_SPAWN
+
+/* Set up ACTIONS and ATTRIBUTES for `posix_spawn'.  Return an error
+   number.  */
+
+static int
+emacs_posix_spawn_init_actions (posix_spawn_file_actions_t *actions,
+                                int std_in, int std_out, int std_err,
+                                const char *cwd)
+{
+  int error = posix_spawn_file_actions_init (actions);
+  if (error != 0)
+    return error;
+
+  error = posix_spawn_file_actions_adddup2 (actions, std_in,
+                                            STDIN_FILENO);
+  if (error != 0)
+    goto out;
+
+  error = posix_spawn_file_actions_adddup2 (actions, std_out,
+                                            STDOUT_FILENO);
+  if (error != 0)
+    goto out;
+
+  error = posix_spawn_file_actions_adddup2 (actions,
+                                            std_err < 0 ? std_out
+                                                        : std_err,
+                                            STDERR_FILENO);
+  if (error != 0)
+    goto out;
+
+  error =
+#ifdef HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR
+    posix_spawn_file_actions_addchdir
+#else
+    posix_spawn_file_actions_addchdir_np
+#endif
+    (actions, cwd);
+  if (error != 0)
+    goto out;
+
+ out:
+  if (error != 0)
+    posix_spawn_file_actions_destroy (actions);
+  return error;
+}
+
+static int
+emacs_posix_spawn_init_attributes (posix_spawnattr_t *attributes)
+{
+  int error = posix_spawnattr_init (attributes);
+  if (error != 0)
+    return error;
+
+  error = posix_spawnattr_setflags (attributes,
+                                    POSIX_SPAWN_SETSID
+                                      | POSIX_SPAWN_SETSIGDEF
+                                      | POSIX_SPAWN_SETSIGMASK);
+  if (error != 0)
+    goto out;
+
+  sigset_t sigdefault;
+  sigemptyset (&sigdefault);
+
+#ifdef DARWIN_OS
+  /* Work around a macOS bug, where SIGCHLD is apparently
+     delivered to a vforked child instead of to its parent.  See:
+     https://lists.gnu.org/r/emacs-devel/2017-05/msg00342.html
+  */
+  sigaddset (&sigdefault, SIGCHLD);
+#endif
+
+  sigaddset (&sigdefault, SIGINT);
+  sigaddset (&sigdefault, SIGQUIT);
+#ifdef SIGPROF
+  sigaddset (&sigdefault, SIGPROF);
+#endif
+
+  /* Emacs ignores SIGPIPE, but the child should not.  */
+  sigaddset (&sigdefault, SIGPIPE);
+  /* Likewise for SIGPROF.  */
+#ifdef SIGPROF
+  sigaddset (&sigdefault, SIGPROF);
+#endif
+
+  error = posix_spawnattr_setsigdefault (attributes, &sigdefault);
+  if (error != 0)
+    goto out;
+
+  /* Stop blocking SIGCHLD in the child.  */
+  sigset_t oldset;
+  error = pthread_sigmask (SIG_SETMASK, NULL, &oldset);
+  if (error != 0)
+    goto out;
+  error = posix_spawnattr_setsigmask (attributes, &oldset);
+  if (error != 0)
+    goto out;
+
+ out:
+  if (error != 0)
+    posix_spawnattr_destroy (attributes);
+
+  return error;
+}
+
+static int
+emacs_posix_spawn_init (posix_spawn_file_actions_t *actions,
+                        posix_spawnattr_t *attributes, int std_in,
+                        int std_out, int std_err, const char *cwd)
+{
+  int error = emacs_posix_spawn_init_actions (actions, std_in,
+                                              std_out, std_err, cwd);
+  if (error != 0)
+    return error;
+
+  error = emacs_posix_spawn_init_attributes (attributes);
+  if (error != 0)
+    return error;
+
+  return 0;
+}
+
+#endif
+
 /* Start a new asynchronous subprocess.  If successful, return zero
    and store the process identifier of the new process in *NEWPID.
    Use STDIN, STDOUT, and STDERR as standard streams for the new
@@ -1228,12 +1366,60 @@ int
 emacs_spawn (pid_t *newpid, int std_in, int std_out, int std_err,
              char **argv, char **envp, const char *cwd, const char *pty)
 {
+#if USABLE_POSIX_SPAWN
+  /* Prefer the simpler `posix_spawn' if available.  `posix_spawn'
+     doesn't yet support setting up pseudoterminals, so we fall back
+     to `vfork' if we're supposed to use a pseudoterminal.  */
+
+  bool use_posix_spawn = pty == NULL;
+
+  posix_spawn_file_actions_t actions;
+  posix_spawnattr_t attributes;
+
+  if (use_posix_spawn)
+    {
+      /* Initialize optional attributes before blocking. */
+      int error
+        = emacs_posix_spawn_init (&actions, &attributes, std_in,
+                                  std_out, std_err, cwd);
+      if (error != 0)
+       return error;
+    }
+#endif
+
   sigset_t oldset;
   int pid;
+  int vfork_error;
 
   block_input ();
   block_child_signal (&oldset);
 
+#if USABLE_POSIX_SPAWN
+  if (use_posix_spawn)
+    {
+      vfork_error = posix_spawn (&pid, argv[0], &actions, &attributes,
+                                 argv, envp);
+      if (vfork_error != 0)
+       pid = -1;
+
+      int error = posix_spawn_file_actions_destroy (&actions);
+      if (error != 0)
+       {
+         errno = error;
+         emacs_perror ("posix_spawn_file_actions_destroy");
+       }
+
+      error = posix_spawnattr_destroy (&attributes);
+      if (error != 0)
+       {
+         errno = error;
+         emacs_perror ("posix_spawnattr_destroy");
+       }
+
+      goto fork_done;
+    }
+#endif
+
 #ifndef WINDOWSNT
   /* vfork, and prevent local vars from being clobbered by the vfork.  */
   pid_t *volatile newpid_volatile = newpid;
@@ -1375,8 +1561,9 @@ emacs_spawn (pid_t *newpid, int std_in, int std_out, int 
std_err,
 
   /* Back in the parent process.  */
 
-  int vfork_error = pid < 0 ? errno : 0;
+  vfork_error = pid < 0 ? errno : 0;
 
+ fork_done:
   /* Stop blocking in the parent.  */
   unblock_child_signal (&oldset);
   unblock_input ();



reply via email to

[Prev in Thread] Current Thread [Next in Thread]