A more-conservative workaround for Cygwin SIGCHLD issues.

* callproc.c (Fcall_process):
* process.c (create_process):
Make sure SIGCHLD is caught before we fork,
since Emacs startup no arranges to catch SIGCHLD.
* process.c (lib_child_handler): Initialize to null, not to
dummy_handler.
(catch_child_signal): Allow self to be called lazily.
Do nothing if it's already been called.
Assume caller has blocked SIGCHLD (all callers do now).
* emacs.c (main): Do not catch SIGCHLD here; defer it until
just before it's really needed.
* nsterm.m (ns_term_init): No need to re-catch SIGCHLD here,
since it hasn't been caught yet.

Fixes: debbugs:14569

5 files changed, 52 insertions, 35 deletions

diff --git a/src/ChangeLog b/src/ChangeLog
index f9451711f32..6357491725d 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,20 @@
+2013-06-23  Paul Eggert  <eggert@cs.ucla.edu>
+
+        A more-conservative workaround for Cygwin SIGCHLD issues (Bug#14569).
+        * callproc.c (Fcall_process):
+        * process.c (create_process):
+        Make sure SIGCHLD is caught before we fork,
+        since Emacs startup no arranges to catch SIGCHLD.
+        * process.c (lib_child_handler): Initialize to null, not to
+        dummy_handler.
+        (catch_child_signal): Allow self to be called lazily.
+        Do nothing if it's already been called.
+        Assume caller has blocked SIGCHLD (all callers do now).
+        * emacs.c (main): Do not catch SIGCHLD here; defer it until
+        just before it's really needed.
+        * nsterm.m (ns_term_init): No need to re-catch SIGCHLD here,
+        since it hasn't been caught yet.
+
 2013-06-23  Lars Magne Ingebrigtsen  <larsi@gnus.org>
 
         * image.c (compute_image_size): New function to implement
diff --git a/src/callproc.c b/src/callproc.c
index f0aa8222342..7db984fa71c 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -613,6 +613,7 @@ usage: (call-process PROGRAM &optional INFILE DESTINATION DISPLAY &rest ARGS)  *
 
     block_input ();
     block_child_signal ();
+    catch_child_signal ();
 
 #ifdef WINDOWSNT
     pid = child_setup (filefd, fd1, fd_error, new_argv, 0, current_dir);
diff --git a/src/emacs.c b/src/emacs.c
index c5b32c7c0e7..13f6d117ebc 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -1257,13 +1257,6 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
     tzset ();
 #endif /* MSDOS */
 
-  /* Do this after initializing the memory allocator, since it uses
-     glib and glib uses malloc.  And do it before anything else that
-     invokes glib, to avoid potential races among glib subthreads in
-     Cygwin glib.  gfilenotify invokes glib, so this can't be delayed
-     further.  */
-  catch_child_signal ();
-
 #ifdef HAVE_GFILENOTIFY
   globals_of_gfilenotify ();
 #endif
diff --git a/src/nsterm.m b/src/nsterm.m
index 93f693fe55e..c88e5034d39 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -4360,12 +4360,6 @@ ns_term_init (Lisp_Object display_name)
 
   [NSApp run];
   ns_do_open_file = YES;
-
-#ifdef NS_IMPL_GNUSTEP
-  /* GNUstep steals SIGCHLD for use in NSTask, but we don't use NSTask.
-     We must re-catch it so subprocess works.  */
-  catch_child_signal ();
-#endif
   return dpyinfo;
 }
 
diff --git a/src/process.c b/src/process.c
index 6df1bf7eff7..3f062b6db16 100644
--- a/src/process.c
+++ b/src/process.c
@@ -1590,7 +1590,6 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
 #ifndef WINDOWSNT
   int wait_child_setup[2];
 #endif
-  sigset_t blocked;
   int forkin, forkout;
   bool pty_flag = 0;
   Lisp_Object lisp_pty_name = Qnil;
@@ -1685,12 +1684,8 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
   encoded_current_dir = ENCODE_FILE (current_dir);
 
   block_input ();
-
-  /* Block SIGCHLD until we have a chance to store the new fork's
-     pid in its process structure.  */
-  sigemptyset (&blocked);
-  sigaddset (&blocked, SIGCHLD);
-  pthread_sigmask (SIG_BLOCK, &blocked, 0);
+  block_child_signal ();
+  catch_child_signal ();
 
 #ifndef WINDOWSNT
   /* vfork, and prevent local vars from being clobbered by the vfork.  */
@@ -1822,8 +1817,8 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
       /* Emacs ignores SIGPIPE, but the child should not.  */
       signal (SIGPIPE, SIG_DFL);
 
-        /* Stop blocking signals in the child.  */
-      pthread_sigmask (SIG_SETMASK, &empty_mask, 0);
+      /* Stop blocking SIGCHLD in the child.  */
+      unblock_child_signal ();
 
       if (pty_flag)
         child_setup_tty (xforkout);
@@ -1843,8 +1838,8 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
   if (pid >= 0)
     XPROCESS (process)->alive = 1;
 
-  /* Stop blocking signals in the parent.  */
-  pthread_sigmask (SIG_SETMASK, &empty_mask, 0);
+  /* Stop blocking in the parent.  */
+  unblock_child_signal ();
   unblock_input ();
 
   if (pid < 0)
@@ -6125,9 +6120,10 @@ process has been transmitted to the serial port.  */)
 
 /* LIB_CHILD_HANDLER is a SIGCHLD handler that Emacs calls while doing
    its own SIGCHLD handling.  On POSIXish systems, glib needs this to
-   keep track of its own children.  The default handler does nothing.  */
+   keep track of its own children.  GNUstep is similar.  */
+
 static void dummy_handler (int sig) {}
-static signal_handler_t volatile lib_child_handler = dummy_handler;
+static signal_handler_t volatile lib_child_handler;
 
 /* Handle a SIGCHLD signal by looking for known child processes of
    Emacs whose status have changed.  For each one found, record its
@@ -7060,7 +7056,10 @@ integer or floating point values.
   return system_process_attributes (pid);
 }
 
-/* Arrange to catch SIGCHLD if needed.  */
+/* Arrange to catch SIGCHLD if this hasn't already been arranged.
+   Invoke this after init_process_emacs, and after glib and/or GNUstep
+   futz with the SIGCHLD handler, but before Emacs forks any children.
+   This function's caller should block SIGCHLD.  */
 
 void
 catch_child_signal (void)
@@ -7072,25 +7071,38 @@ catch_child_signal (void)
     return;
 #endif
 
+#ifndef NS_IMPL_GNUSTEP
+  if (lib_child_handler)
+    return;
+#endif
+
 #if defined HAVE_GLIB && !defined WINDOWSNT
   /* Tickle glib's child-handling code.  Ask glib to wait for Emacs itself;
      this should always fail, but is enough to initialize glib's
      private SIGCHLD handler, allowing the code below to copy it into
      LIB_CHILD_HANDLER.
 
-     Do this early in Emacs initialization, before glib creates
-     threads, to avoid race condition bugs in Cygwin glib.  */
-  g_source_unref (g_child_watch_source_new (getpid ()));
+     Do this here, rather than early in Emacs initialization where it
+     might make more sense, to try to avoid bugs in Cygwin glib (Bug#14569).  */
+  {
+    GSource *source = g_child_watch_source_new (getpid ());
+    g_source_unref (source);
+  }
 #endif
 
   emacs_sigaction_init (&action, deliver_child_signal);
-  block_child_signal ();
   sigaction (SIGCHLD, &action, &old_action);
   eassert (! (old_action.sa_flags & SA_SIGINFO));
-  if (old_action.sa_handler != SIG_DFL && old_action.sa_handler != SIG_IGN
-      && old_action.sa_handler != deliver_child_signal)
-    lib_child_handler = old_action.sa_handler;
-  unblock_child_signal ();
+
+#ifdef NS_IMPL_GNUSTEP
+  if (old_action.sa_handler == deliver_child_signal)
+    return;
+#endif
+
+  lib_child_handler
+    = (old_action.sa_handler == SIG_DFL || old_action.sa_handler == SIG_IGN
+       ? dummy_handler
+       : old_action.sa_handler);
 }