Re: SIGCHLD in vfork child context

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

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

Andreas Schwab wrote:
>> Which tree should we be barking up instead?
> Don't abort.

Good suggestion, thanks. I installed the attached patch to try to bark up the 
right tree, or to chew up the tree bark, or whatever....


[-- Attachment #2: 0001-Attempt-to-work-around-macOS-vfork-bug.patch --]
[-- Type: text/x-patch, Size: 4896 bytes --]

From 7c951fd51832badb09055a8e177f8ec358cbbdcf Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 19 May 2017 00:11:48 -0700
Subject: [PATCH] Attempt to work around macOS vfork bug

Problem reported by YAMAMOTO Mitsuharu in:
http://lists.gnu.org/archive/html/emacs-devel/2017-05/msg00342.html
This is related to the fix for Bug#26397.
* src/callproc.c (call_process_cleanup, call_process) [!MSDOS]:
Report internal error if wait_for_termination fails.
* src/sysdep.c (get_child_status): Return -1 if waitpid is
buggy, instead of aborting.
(wait_for_termination): Return bool success value.
All callers changed.
---
 src/callproc.c | 13 +++++++++----
 src/sysdep.c   | 25 ++++++++++++++-----------
 src/syswait.h  |  2 +-
 3 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/src/callproc.c b/src/callproc.c
index e967e45..7c85eed 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -202,10 +202,11 @@ call_process_cleanup (Lisp_Object buffer)
       message1 ("Waiting for process to die...(type C-g again to kill it instantly)");
 
       /* This will quit on C-g.  */
-      wait_for_termination (synch_process_pid, 0, 1);
-
+      bool wait_ok = wait_for_termination (synch_process_pid, NULL, true);
       synch_process_pid = 0;
-      message1 ("Waiting for process to die...done");
+      message1 (wait_ok
+		? "Waiting for process to die...done"
+		: "Waiting for process to die...internal error");
     }
 #endif	/* !MSDOS */
 }
@@ -866,9 +867,10 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
 	       make_number (total_read));
     }
 
+  bool wait_ok = true;
 #ifndef MSDOS
   /* Wait for it to terminate, unless it already has.  */
-  wait_for_termination (pid, &status, fd0 < 0);
+  wait_ok = wait_for_termination (pid, &status, fd0 < 0);
 #endif
 
   /* Don't kill any children that the subprocess may have left behind
@@ -878,6 +880,9 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
   SAFE_FREE ();
   unbind_to (count, Qnil);
 
+  if (!wait_ok)
+    return build_unibyte_string ("internal error");
+
   if (WIFSIGNALED (status))
     {
       const char *signame;
diff --git a/src/sysdep.c b/src/sysdep.c
index ac6eed0..70f4a9d 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -368,8 +368,8 @@ init_baud_rate (int fd)
    Use waitpid-style OPTIONS when waiting.
    If INTERRUPTIBLE, this function is interruptible by a signal.
 
-   Return CHILD if successful, 0 if no status is available;
-   the latter is possible only when options & NOHANG.  */
+   Return CHILD if successful, 0 if no status is available, and a
+   negative value (setting errno) if waitpid is buggy.  */
 static pid_t
 get_child_status (pid_t child, int *status, int options, bool interruptible)
 {
@@ -392,13 +392,14 @@ get_child_status (pid_t child, int *status, int options, bool interruptible)
       pid = waitpid (child, status, options);
       if (0 <= pid)
 	break;
-
-      /* Check that CHILD is a child process that has not been reaped,
-	 and that STATUS and OPTIONS are valid.  Otherwise abort,
-	 as continuing after this internal error could cause Emacs to
-	 become confused and kill innocent-victim processes.  */
       if (errno != EINTR)
-	emacs_abort ();
+	{
+	  /* Most likely, waitpid is buggy and the operating system
+	     lost track of the child somehow.  Return -1 and let the
+	     caller try to figure things out.  Possibly the bug could
+	     cause Emacs to kill the wrong process.  Oh well.  */
+	  return pid;
+	}
     }
 
   /* If successful and status is requested, tell wait_reading_process_output
@@ -413,11 +414,13 @@ get_child_status (pid_t child, int *status, int options, bool interruptible)
    CHILD must be a child process that has not been reaped.
    If STATUS is non-null, store the waitpid-style exit status into *STATUS
    and tell wait_reading_process_output that it needs to look around.
-   If INTERRUPTIBLE, this function is interruptible by a signal.  */
-void
+   If INTERRUPTIBLE, this function is interruptible by a signal.
+   Return true if successful, false (setting errno) if CHILD cannot be
+   waited for because waitpid is buggy.  */
+bool
 wait_for_termination (pid_t child, int *status, bool interruptible)
 {
-  get_child_status (child, status, 0, interruptible);
+  return 0 <= get_child_status (child, status, 0, interruptible);
 }
 
 /* Report whether the subprocess with process id CHILD has changed status.
diff --git a/src/syswait.h b/src/syswait.h
index 846a975..055562a 100644
--- a/src/syswait.h
+++ b/src/syswait.h
@@ -56,7 +56,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #endif
 
 /* Defined in sysdep.c.  */
-extern void wait_for_termination (pid_t, int *, bool);
+extern bool wait_for_termination (pid_t, int *, bool);
 extern pid_t child_status_changed (pid_t, int *, int);
 
 #endif /* EMACS_SYSWAIT_H */
-- 
2.7.4