>From 53bbb62f77a652eac84c2e4424d412f25fa53f97 Mon Sep 17 00:00:00 2001 From: Noam Postavsky Date: Thu, 20 Sep 2018 20:17:51 -0400 Subject: [PATCH] Revert vfork for Darwin changes Reverts the following commits: [1: a13eaddce2]: 2017-04-18 11:42:30 +0100 Use vfork if possible on Darwin (bug#26397) https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=a13eaddce2ddbe3ba0b7f4c81715bc0fcdba99f6 [2: 709259dcc5]: 2017-05-17 10:59:02 -0700 Work around AddressSanitizer bug with vfork https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=709259dcc501ef991991a35a6ffb2aef02a62c60 [3: 7c951fd518]: 2017-05-19 00:13:27 -0700 Attempt to work around macOS vfork bug https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=7c951fd51832badb09055a8e177f8ec358cbbdcf [4: cb6d669744]: 2017-05-21 01:47:31 -0700 Work around macOS bug with vforked child https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=cb6d66974416f535fefb42c974b73037e257399a [5: 9759b249e9]: 2017-05-21 02:00:29 -0700 Work around macOS bug in create_process, too https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9759b249e97d4b05644309fc70ae9277b347027e --- src/callproc.c | 34 ++++------------------------------ src/conf_post.h | 12 ++++++------ src/process.c | 14 -------------- src/sysdep.c | 25 +++++++++++-------------- src/syswait.h | 2 +- 5 files changed, 22 insertions(+), 65 deletions(-) diff --git a/src/callproc.c b/src/callproc.c index 973f324139..10f1ce8ee8 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -52,8 +52,6 @@ your option) any later version. #include "syswait.h" #include "blockinput.h" #include "frame.h" -#include "systty.h" -#include "keyboard.h" #ifdef MSDOS #include "msdos.h" @@ -202,11 +200,10 @@ 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. */ - bool wait_ok = wait_for_termination (synch_process_pid, NULL, true); + wait_for_termination (synch_process_pid, 0, 1); + synch_process_pid = 0; - message1 (wait_ok - ? "Waiting for process to die...done" - : "Waiting for process to die...internal error"); + message1 ("Waiting for process to die...done"); } #endif /* !MSDOS */ } @@ -631,28 +628,9 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd, if (pid == 0) { -#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 - */ - signal (SIGCHLD, SIG_DFL); -#endif - unblock_child_signal (&oldset); -#ifdef DARWIN_OS - /* Darwin doesn't let us run setsid after a vfork, so use - TIOCNOTTY when necessary. */ - int j = emacs_open (DEV_TTY, O_RDWR, 0); - if (j >= 0) - { - ioctl (j, TIOCNOTTY, 0); - emacs_close (j); - } -#else setsid (); -#endif /* Emacs ignores SIGPIPE, but the child should not. */ signal (SIGPIPE, SIG_DFL); @@ -875,10 +853,9 @@ 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_ok = wait_for_termination (pid, &status, fd0 < 0); + wait_for_termination (pid, &status, fd0 < 0); #endif /* Don't kill any children that the subprocess may have left behind @@ -888,9 +865,6 @@ 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/conf_post.h b/src/conf_post.h index 69f686d72d..7efb0a0d32 100644 --- a/src/conf_post.h +++ b/src/conf_post.h @@ -101,6 +101,12 @@ your option) any later version. #define realloc unexec_realloc #define free unexec_free #endif +/* The following solves the problem that Emacs hangs when evaluating + (make-comint "test0" "/nodir/nofile" nil "") when /nodir/nofile + does not exist. Also, setsid is not allowed in the vfork child's + context as of Darwin 9/Mac OS X 10.5. */ +#undef HAVE_WORKING_VFORK +#define vfork fork #endif /* DARWIN_OS */ /* If HYBRID_MALLOC is defined (e.g., on Cygwin), emacs will use @@ -338,12 +344,6 @@ your option) any later version. # define ATTRIBUTE_NO_SANITIZE_ADDRESS #endif -/* gcc -fsanitize=address does not work with vfork in Fedora 25 x86-64. - For now, assume that this problem occurs on all platforms. */ -#if ADDRESS_SANITIZER && !defined vfork -# define vfork fork -#endif - /* Some versions of GNU/Linux define noinline in their headers. */ #ifdef noinline #undef noinline diff --git a/src/process.c b/src/process.c index b0a327229c..80ddd3518c 100644 --- a/src/process.c +++ b/src/process.c @@ -2056,21 +2056,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; diff --git a/src/sysdep.c b/src/sysdep.c index 34bff23386..f4038bb84a 100644 --- a/src/sysdep.c +++ b/src/sysdep.c @@ -422,8 +422,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, and a - negative value (setting errno) if waitpid is buggy. */ + Return CHILD if successful, 0 if no status is available; + the latter is possible only when options & NOHANG. */ static pid_t get_child_status (pid_t child, int *status, int options, bool interruptible) { @@ -446,14 +446,13 @@ 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) - { - /* 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; - } + emacs_abort (); } /* If successful and status is requested, tell wait_reading_process_output @@ -468,13 +467,11 @@ 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. - Return true if successful, false (setting errno) if CHILD cannot be - waited for because waitpid is buggy. */ -bool + If INTERRUPTIBLE, this function is interruptible by a signal. */ +void wait_for_termination (pid_t child, int *status, bool interruptible) { - return 0 <= get_child_status (child, status, 0, interruptible); + 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 8f0357f9d6..4af75f037e 100644 --- a/src/syswait.h +++ b/src/syswait.h @@ -56,7 +56,7 @@ your option) any later version. #endif /* Defined in sysdep.c. */ -extern bool wait_for_termination (pid_t, int *, bool); +extern void wait_for_termination (pid_t, int *, bool); extern pid_t child_status_changed (pid_t, int *, int); #endif /* EMACS_SYSWAIT_H */ -- 2.11.0