Fix race conditions with signal handlers and errno.

Be more systematic about preserving errno whenever a signal
handler returns, even if it's not in the main thread.  Do this by
renaming signal handlers to distinguish between signal delivery
and signal handling.  All uses changed.
* atimer.c (deliver_alarm_signal): Rename from alarm_signal_handler.
* data.c (deliver_arith_signal): Rename from arith_error.
* dispnew.c (deliver_window_change_signal): Rename from
window_change_signal.
* emacs.c (deliver_error_signal): Rename from fatal_error_signal.
(deliver_danger_signal) [SIGDANGER]: Rename from memory_warning_signal.
* keyboard.c (deliver_input_available_signal): Rename from
input_available_signal.
(deliver_user_signal): Rename from handle_user_signal.
(deliver_interrupt_signal): Rename from interrupt_signal.
* process.c (deliver_pipe_signal): Rename from send_process_trap.
(deliver_child_signal): Rename from sigchld_handler.
* atimer.c (handle_alarm_signal):
* data.c (handle_arith_signal):
* dispnew.c (handle_window_change_signal):
* emacs.c (handle_fatal_signal, handle_danger_signal):
* keyboard.c (handle_input_available_signal):
* keyboard.c (handle_user_signal, handle_interrupt_signal):
* process.c (handle_pipe_signal, handle_child_signal):
New functions, with the actual signal-handling code taken from the
original respective signal handlers, sans the sporadic attempts to
preserve errno, since that's now done by handle_on_main_thread.
* atimer.c (alarm_signal_handler): Remove unnecessary decl.
* emacs.c, floatfns.c, lisp.h: Remove unused FLOAT_CATCH_SIGKILL cruft.
* emacs.c (main_thread) [FORWARD_SIGNAL_TO_MAIN_THREAD]:
Move to sysdep.c.
(main) [FORWARD_SIGNAL_TO_MAIN_THREAD]:
Move initialization of main_thread to sysdep.c's init_signals.
* process.c (waitpid) [!WNOHANG]: #define to wait; that's good enough for
our usage, and simplifies the mainline code.
(record_child_status_change): New static function, as a helper
for handle_child_signal, and with most of the old child handler's
contents.
(CAN_HANDLE_MULTIPLE_CHILDREN): New constant.
(handle_child_signal): Use the above.
* sysdep.c (main_thread) [FORWARD_SIGNAL_TO_MAIN_THREAD]:
Moved here from emacs.c.
(init_signals) [FORWARD_SIGNAL_TO_MAIN_THREAD]: Initialize it;
code moved here from emacs.c's main function.
* sysdep.c, syssignal.h (handle_on_main_thread): New function,
replacing the old SIGNAL_THREAD_CHECK.  All uses changed.  This
lets callers save and restore errno properly.

1 files changed, 129 insertions, 127 deletions

diff --git a/src/process.c b/src/process.c
index 5677da36881..3a6615fb505 100644
--- a/src/process.c
+++ b/src/process.c
@@ -124,6 +124,14 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "xgselect.h"
 #endif
 
+#ifndef WNOHANG
+# undef waitpid
+# define waitpid(pid, status, options) wait (status)
+#endif
+#ifndef WUNTRACED
+# define WUNTRACED 0
+#endif
+
 /* Work around GCC 4.7.0 bug with strict overflow checking; see
    <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52904>.
    These lines can be removed once the GCC bug is fixed.  */
@@ -801,7 +809,7 @@ get_process (register Lisp_Object name)
 #ifdef SIGCHLD
 /* Fdelete_process promises to immediately forget about the process, but in
    reality, Emacs needs to remember those processes until they have been
-   treated by sigchld_handler; otherwise this handler would consider the
+   treated by the SIGCHLD handler; otherwise this handler would consider the
    process as being synchronous and say that the synchronous process is
    dead.  */
 static Lisp_Object deleted_pid_list;
@@ -849,7 +857,8 @@ nil, indicating the current buffer's process.  */)
 #endif
         {
           Fkill_process (process, Qnil);
-          /* Do this now, since remove_process will make sigchld_handler do nothing.  */
+          /* Do this now, since remove_process will make the
+             SIGCHLD handler do nothing.  */
           pset_status (p, Fcons (Qsignal, Fcons (make_number (SIGKILL), Qnil)));
           p->tick = ++process_tick;
           status_notify (p);
@@ -1728,7 +1737,7 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
   if (inchannel > max_process_desc)
     max_process_desc = inchannel;
 
-  /* Until we store the proper pid, enable sigchld_handler
+  /* Until we store the proper pid, enable the SIGCHLD handler
      to recognize an unknown pid as standing for this process.
      It is very important not to let this `marker' value stay
      in the table after this function has returned; if it does
@@ -4956,8 +4965,8 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 
                   if (p->pid == -2)
                     {
-                      /* If the EIO occurs on a pty, sigchld_handler's
-                         waitpid() will not find the process object to
+                      /* If the EIO occurs on a pty, the SIGCHLD handler's
+                         waitpid call will not find the process object to
                          delete.  Do it here.  */
                       p->tick = ++process_tick;
                       pset_status (p, Qfailed);
@@ -5422,18 +5431,19 @@ read_process_output (Lisp_Object proc, register int channel)
 static jmp_buf send_process_frame;
 static Lisp_Object process_sent_to;
 
-#ifndef FORWARD_SIGNAL_TO_MAIN_THREAD
-static _Noreturn void send_process_trap (int);
-#endif
-
-static void
-send_process_trap (int ignore)
+static _Noreturn void
+handle_pipe_signal (int sig)
 {
-  SIGNAL_THREAD_CHECK (SIGPIPE);
   sigunblock (sigmask (SIGPIPE));
   _longjmp (send_process_frame, 1);
 }
 
+static void
+deliver_pipe_signal (int sig)
+{
+  handle_on_main_thread (sig, handle_pipe_signal);
+}
+
 /* In send_process, when a write fails temporarily,
    wait_reading_process_output is called.  It may execute user code,
    e.g. timers, that attempts to write new data to the same process.
@@ -5663,7 +5673,7 @@ send_process (volatile Lisp_Object proc, const char *volatile buf,
               /* Send this batch, using one or more write calls.  */
               ptrdiff_t written = 0;
               int outfd = p->outfd;
-              old_sigpipe = (void (*) (int)) signal (SIGPIPE, send_process_trap);
+              old_sigpipe = signal (SIGPIPE, deliver_pipe_signal);
 #ifdef DATAGRAM_SOCKETS
               if (DATAGRAM_CHAN_P (outfd))
                 {
@@ -6397,143 +6407,135 @@ process has been transmitted to the serial port.  */)
    indirectly; if it does, that is a bug  */
 
 #ifdef SIGCHLD
-static void
-sigchld_handler (int signo)
+
+/* Record one child's changed status.  Return true if a child was found.  */
+static bool
+record_child_status_change (void)
 {
-  int old_errno = errno;
   Lisp_Object proc;
   struct Lisp_Process *p;
+  pid_t pid;
+  int w;
+  Lisp_Object tail;
 
-  SIGNAL_THREAD_CHECK (signo);
-
-  while (1)
-    {
-      pid_t pid;
-      int w;
-      Lisp_Object tail;
-
-#ifdef WNOHANG
-#ifndef WUNTRACED
-#define WUNTRACED 0
-#endif /* no WUNTRACED */
-      /* Keep trying to get a status until we get a definitive result.  */
-      do
-        {
-          errno = 0;
-          pid = waitpid (-1, &w, WNOHANG | WUNTRACED);
-        }
-      while (pid < 0 && errno == EINTR);
-
-      if (pid <= 0)
-        {
-          /* PID == 0 means no processes found, PID == -1 means a real
-             failure.  We have done all our job, so return.  */
+  do
+    pid = waitpid (-1, &w, WNOHANG | WUNTRACED);
+  while (pid < 0 && errno == EINTR);
 
-          errno = old_errno;
-          return;
-        }
-#else
-      pid = wait (&w);
-#endif /* no WNOHANG */
+  /* PID == 0 means no processes found, PID == -1 means a real failure.
+     Either way, we have done all our job.  */
+  if (pid <= 0)
+    return false;
 
-      /* Find the process that signaled us, and record its status.  */
+  /* Find the process that signaled us, and record its status.  */
 
-      /* The process can have been deleted by Fdelete_process.  */
-      for (tail = deleted_pid_list; CONSP (tail); tail = XCDR (tail))
+  /* The process can have been deleted by Fdelete_process.  */
+  for (tail = deleted_pid_list; CONSP (tail); tail = XCDR (tail))
+    {
+      Lisp_Object xpid = XCAR (tail);
+      if ((INTEGERP (xpid) && pid == XINT (xpid))
+          || (FLOATP (xpid) && pid == XFLOAT_DATA (xpid)))
         {
-          Lisp_Object xpid = XCAR (tail);
-          if ((INTEGERP (xpid) && pid == XINT (xpid))
-              || (FLOATP (xpid) && pid == XFLOAT_DATA (xpid)))
-            {
-              XSETCAR (tail, Qnil);
-              goto sigchld_end_of_loop;
-            }
+          XSETCAR (tail, Qnil);
+          return true;
         }
+    }
 
-      /* Otherwise, if it is asynchronous, it is in Vprocess_alist.  */
+  /* Otherwise, if it is asynchronous, it is in Vprocess_alist.  */
+  p = 0;
+  for (tail = Vprocess_alist; CONSP (tail); tail = XCDR (tail))
+    {
+      proc = XCDR (XCAR (tail));
+      p = XPROCESS (proc);
+      if (EQ (p->type, Qreal) && p->pid == pid)
+        break;
       p = 0;
-      for (tail = Vprocess_alist; CONSP (tail); tail = XCDR (tail))
-        {
-          proc = XCDR (XCAR (tail));
-          p = XPROCESS (proc);
-          if (EQ (p->type, Qreal) && p->pid == pid)
-            break;
-          p = 0;
-        }
-
-      /* Look for an asynchronous process whose pid hasn't been filled
-         in yet.  */
-      if (p == 0)
-        for (tail = Vprocess_alist; CONSP (tail); tail = XCDR (tail))
-          {
-            proc = XCDR (XCAR (tail));
-            p = XPROCESS (proc);
-            if (p->pid == -1)
-              break;
-            p = 0;
-          }
-
-      /* Change the status of the process that was found.  */
-      if (p != 0)
-        {
-          int clear_desc_flag = 0;
+    }
 
-          p->tick = ++process_tick;
-          p->raw_status = w;
-          p->raw_status_new = 1;
+  /* Look for an asynchronous process whose pid hasn't been filled
+     in yet.  */
+  if (! p)
+    for (tail = Vprocess_alist; CONSP (tail); tail = XCDR (tail))
+      {
+        proc = XCDR (XCAR (tail));
+        p = XPROCESS (proc);
+        if (p->pid == -1)
+          break;
+        p = 0;
+      }
 
-          /* If process has terminated, stop waiting for its output.  */
-          if ((WIFSIGNALED (w) || WIFEXITED (w))
-              && p->infd >= 0)
-            clear_desc_flag = 1;
+  /* Change the status of the process that was found.  */
+  if (p)
+    {
+      int clear_desc_flag = 0;
 
-          /* We use clear_desc_flag to avoid a compiler bug in Microsoft C.  */
-          if (clear_desc_flag)
-            {
-              FD_CLR (p->infd, &input_wait_mask);
-              FD_CLR (p->infd, &non_keyboard_wait_mask);
-            }
+      p->tick = ++process_tick;
+      p->raw_status = w;
+      p->raw_status_new = 1;
 
-          /* Tell wait_reading_process_output that it needs to wake up and
-             look around.  */
-          if (input_available_clear_time)
-            *input_available_clear_time = make_emacs_time (0, 0);
-        }
+      /* If process has terminated, stop waiting for its output.  */
+      if ((WIFSIGNALED (w) || WIFEXITED (w))
+          && p->infd >= 0)
+        clear_desc_flag = 1;
 
-      /* There was no asynchronous process found for that pid: we have
-         a synchronous process.  */
-      else
+      /* We use clear_desc_flag to avoid a compiler bug in Microsoft C.  */
+      if (clear_desc_flag)
         {
-          synch_process_alive = 0;
-
-          /* Report the status of the synchronous process.  */
-          if (WIFEXITED (w))
-            synch_process_retcode = WEXITSTATUS (w);
-          else if (WIFSIGNALED (w))
-            synch_process_termsig = WTERMSIG (w);
-
-          /* Tell wait_reading_process_output that it needs to wake up and
-             look around.  */
-          if (input_available_clear_time)
-            *input_available_clear_time = make_emacs_time (0, 0);
+          FD_CLR (p->infd, &input_wait_mask);
+          FD_CLR (p->infd, &non_keyboard_wait_mask);
         }
 
-    sigchld_end_of_loop:
-      ;
+      /* Tell wait_reading_process_output that it needs to wake up and
+         look around.  */
+      if (input_available_clear_time)
+        *input_available_clear_time = make_emacs_time (0, 0);
+    }
+  /* There was no asynchronous process found for that pid: we have
+     a synchronous process.  */
+  else
+    {
+      synch_process_alive = 0;
+
+      /* Report the status of the synchronous process.  */
+      if (WIFEXITED (w))
+        synch_process_retcode = WEXITSTATUS (w);
+      else if (WIFSIGNALED (w))
+        synch_process_termsig = WTERMSIG (w);
+
+      /* Tell wait_reading_process_output that it needs to wake up and
+         look around.  */
+      if (input_available_clear_time)
+        *input_available_clear_time = make_emacs_time (0, 0);
+    }
+
+  return true;
+}
 
-      /* On some systems, we must return right away.
-         If any more processes want to signal us, we will
-         get another signal.
-         Otherwise (on systems that have WNOHANG), loop around
-         to use up all the processes that have something to tell us.  */
+/* On some systems, the SIGCHLD handler must return right away.  If
+   any more processes want to signal us, we will get another signal.
+   Otherwise, loop around to use up all the processes that have
+   something to tell us.  */
 #if (defined WINDOWSNT \
      || (defined USG && !defined GNU_LINUX \
          && !(defined HPUX && defined WNOHANG)))
-      errno = old_errno;
-      return;
-#endif /* USG, but not HPUX with WNOHANG */
-    }
+enum { CAN_HANDLE_MULTIPLE_CHILDREN = 1 };
+#else
+enum { CAN_HANDLE_MULTIPLE_CHILDREN = 0 };
+#endif
+
+static void
+handle_child_signal (int sig)
+{
+  while (record_child_status_change () && CAN_HANDLE_MULTIPLE_CHILDREN)
+    continue;
 }
+
+static void
+deliver_child_signal (int sig)
+{
+  handle_on_main_thread (sig, handle_child_signal);
+}
+
 #endif /* SIGCHLD */
 
 
@@ -7387,7 +7389,7 @@ init_process_emacs (void)
 #ifndef CANNOT_DUMP
   if (! noninteractive || initialized)
 #endif
-    signal (SIGCHLD, sigchld_handler);
+    signal (SIGCHLD, deliver_child_signal);
 #endif
 
   FD_ZERO (&input_wait_mask);