bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 251 bytes --]

> The main block of this code looks identical to the BSD specific code
> just underneath (#ifdef TIOCNOTTY), is there a reason we can’t reuse
> that?

I don't see why not. Proposed patch attached; please give it a try as I 
don't use macOS.


[-- Attachment #2: 0001-Let-macOS-vfork-to-create-processes.patch --]
[-- Type: text/x-patch, Size: 4209 bytes --]

From c72e828018b68ed1ce1cad9205b6037f86112a28 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 5 Nov 2018 09:25:46 -0800
Subject: [PATCH] Let macOS vfork to create processes

* src/process.c (create_process) [DARWIN_OS]:
Use vfork, not fork.  However, do not use setsid;
instead, rely on TIOCSPGRP.  Move SIGCHLD ignoring
to its logical place next to the ignoring of other signals.
---
 src/process.c | 51 +++++++++++++++++++++++----------------------------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/src/process.c b/src/process.c
index 6cda4f27ac..dd8beed117 100644
--- a/src/process.c
+++ b/src/process.c
@@ -2066,21 +2066,7 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
   int volatile forkerr_volatile = forkerr;
   struct Lisp_Process *p_volatile = p;
 
-#ifdef DARWIN_OS
-  /* Darwin doesn't let us run setsid after a vfork, so use fork when
-     necessary.  Also, reset SIGCHLD handling after a vfork, as
-     apparently macOS can mistakenly deliver SIGCHLD to the child.  */
-  if (pty_flag)
-    pid = fork ();
-  else
-    {
-      pid = vfork ();
-      if (pid == 0)
-	signal (SIGCHLD, SIG_DFL);
-    }
-#else
   pid = vfork ();
-#endif
 
   current_dir = current_dir_volatile;
   lisp_pty_name = lisp_pty_name_volatile;
@@ -2098,18 +2084,22 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
       /* Make the pty be the controlling terminal of the process.  */
 #ifdef HAVE_PTYS
       /* First, disconnect its current controlling terminal.
-	 Do this even if !PTY_FLAG; see Bug#30762.  */
+	 Do this even if !PTY_FLAG; see Bug#30762.
+	 However, Darwin should not invoke setsid after vfork (see
+	 Bug#33050 and Bug#33154), so rely TIOCNOTTY below instead.  */
+# ifndef DARWIN_OS
       setsid ();
+# endif
       /* Make the pty's terminal the controlling terminal.  */
       if (pty_flag && forkin >= 0)
 	{
-#ifdef TIOCSCTTY
+# ifdef TIOCSCTTY
 	  /* We ignore the return value
 	     because faith@cs.unc.edu says that is necessary on Linux.  */
 	  ioctl (forkin, TIOCSCTTY, 0);
-#endif
+# endif
 	}
-#if defined (LDISC1)
+# ifdef LDISC1
       if (pty_flag && forkin >= 0)
 	{
 	  struct termios t;
@@ -2118,17 +2108,15 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
 	  if (tcsetattr (forkin, TCSANOW, &t) < 0)
 	    emacs_perror ("create_process/tcsetattr LDISC1");
 	}
-#else
-#if defined (NTTYDISC) && defined (TIOCSETD)
+# elif defined NTTYDISC && defined TIOCSETD
       if (pty_flag && forkin >= 0)
 	{
 	  /* Use new line discipline.  */
 	  int ldisc = NTTYDISC;
 	  ioctl (forkin, TIOCSETD, &ldisc);
 	}
-#endif
-#endif
-#ifdef TIOCNOTTY
+# endif
+# ifdef TIOCNOTTY
       /* In 4.3BSD, the TIOCSPGRP bug has been fixed, and now you
 	 can do TIOCSPGRP only to the process's controlling tty.  */
       if (pty_flag)
@@ -2142,9 +2130,9 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
 	      emacs_close (j);
 	    }
 	}
-#endif /* TIOCNOTTY */
+# endif
 
-#if !defined (DONT_REOPEN_PTY)
+# ifndef DONT_REOPEN_PTY
 /*** There is a suggestion that this ought to be a
      conditional on TIOCSPGRP, or !defined TIOCSCTTY.
      Trying the latter gave the wrong results on Debian GNU/Linux 1.1;
@@ -2168,14 +2156,14 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
 	    }
 
 	}
-#endif /* not DONT_REOPEN_PTY */
+# endif
 
-#ifdef SETUP_SLAVE_PTY
+# ifdef SETUP_SLAVE_PTY
       if (pty_flag)
 	{
 	  SETUP_SLAVE_PTY;
 	}
-#endif /* SETUP_SLAVE_PTY */
+# endif
 #endif /* HAVE_PTYS */
 
       signal (SIGINT, SIG_DFL);
@@ -2187,6 +2175,13 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
       /* Emacs ignores SIGPIPE, but the child should not.  */
       signal (SIGPIPE, SIG_DFL);
 
+#ifdef DARWIN_OS
+      /* https://lists.gnu.org/r/emacs-devel/2017-05/msg00342.html
+	 says macOS has a bug where SIGCHLD is delivered to a vforked
+	 child instead of to its parent.  */
+      signal (SIGCHLD, SIG_DFL);
+#endif
+
       /* Stop blocking SIGCHLD in the child.  */
       unblock_child_signal (&oldset);
 
-- 
2.19.1