[Emacs-diffs] trunk r113148: A more-conservative workaround for Cygwin SIGCHLD issues.

[Paul Eggert]

------------------------------------------------------------
revno: 113148
revision-id: address@hidden
parent: address@hidden
fixes bug: http://debbugs.gnu.org/14569
committer: Paul Eggert <address@hidden>
branch nick: trunk
timestamp: Sun 2013-06-23 17:31:31 -0700
message:
  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.
modified:
  src/ChangeLog                  changelog-20091113204419-o5vbwnq5f7feedwu-1438
  src/callproc.c                 callproc.c-20091113204419-o5vbwnq5f7feedwu-248
  src/emacs.c                    emacs.c-20091113204419-o5vbwnq5f7feedwu-241
  src/nsterm.m                   nsterm.m-20091113204419-o5vbwnq5f7feedwu-8747
  src/process.c                  process.c-20091113204419-o5vbwnq5f7feedwu-462

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2013-06-23 19:24:27 +0000
+++ b/src/ChangeLog     2013-06-24 00:31:31 +0000
@@ -1,3 +1,20 @@
+2013-06-23  Paul Eggert  <address@hidden>
+
+       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  <address@hidden>
 
        * image.c (compute_image_size): New function to implement

=== modified file 'src/callproc.c'
--- a/src/callproc.c    2013-06-23 18:18:47 +0000
+++ b/src/callproc.c    2013-06-24 00:31:31 +0000
@@ -613,6 +613,7 @@
 
     block_input ();
     block_child_signal ();
+    catch_child_signal ();
 
 #ifdef WINDOWSNT
     pid = child_setup (filefd, fd1, fd_error, new_argv, 0, current_dir);

=== modified file 'src/emacs.c'
--- a/src/emacs.c       2013-06-23 18:18:47 +0000
+++ b/src/emacs.c       2013-06-24 00:31:31 +0000
@@ -1257,13 +1257,6 @@
     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

=== modified file 'src/nsterm.m'
--- a/src/nsterm.m      2013-06-03 18:29:30 +0000
+++ b/src/nsterm.m      2013-06-24 00:31:31 +0000
@@ -4360,12 +4360,6 @@
 
   [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;
 }
 

=== modified file 'src/process.c'
--- a/src/process.c     2013-06-23 18:18:47 +0000
+++ b/src/process.c     2013-06-24 00:31:31 +0000
@@ -1590,7 +1590,6 @@
 #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 @@
   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 @@
       /* 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 @@
   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 @@
 
 /* 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 @@
   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 @@
     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);
 }