bug#14474: 24.3.50; Zombie subprocesses (again)

[Paul Eggert]

I found another problem with trying to have Emacs use glib's child watcher.
glib's signal handling code uses SA_RESTART and SA_NOCLDSTOP.
Both flags are non-starters for Emacs.  SA_NOCLDSTOP, I suppose,
could be conditionalized based on the discussion in Gnome bug
reports 701538 and 562501.  But SA_RESTART is more of a worry.
An interactive Emacs doesn't want SA_RESTART, because Emacs wants
long-running syscalls to be interrupted after a signal, not
restarted.

I thought of a way to work around this problem: have Emacs catch
SIGCHLD using its own flags, and call glib's SIGCHLD handler as part
of Emacs's SIGCHLD handler.  So I installed the patch quoted at the
end of this message into the Emacs trunk as bzr 112859.  If you've
had D-bus problems please try this new approach.

This raises three more questions for glib, though.  First,
why does glib use SA_RESTART?  If it's to avoid having application
syscalls fail with errno==EINTR, then we're OK.  But if it's to
avoid having glib's internal syscalls fail with errno==EINTR, then
we have a problem, as that can happen with the following patch
(and it can also happen with vanilla Emacs 24.3).

Second, should there be a more robust way for Emacs to invoke
glib's SIGCHLD handler.  The code below is a bit of a hack:
it uses g_source_unref (g_child_watch_source_new (0)) to
create and free a dummy SIGCHLD source, the only reason being
to trick glib into installing its SIGCHLD handler.  It also assumes
that glib does not use SA_SIGINFO.  This all seems fairly fragile.

Third, if a glib memory allocation fails, what does Emacs do?
Emacs tries hard not to exit when there's a memory allocation failure,
but I worry that glib will simply call 'exit' if malloc fails, which
is not good.

=== modified file 'src/ChangeLog'
--- src/ChangeLog       2013-06-05 12:17:02 +0000
+++ src/ChangeLog       2013-06-05 17:04:13 +0000
@@ -1,3 +1,17 @@
+2013-06-05  Paul Eggert  <eggert@cs.ucla.edu>
+
+       Chain glib's SIGCHLD handler from Emacs's (Bug#14474).
+       * process.c (dummy_handler): New function.
+       (lib_child_handler): New static var.
+       (handle_child_signal): Invoke it.
+       (catch_child_signal): If a library has set up a signal handler,
+       save it into lib_child_handler.
+       (init_process_emacs): If using glib and not on Windows, tickle glib's
+       child-handling code so that it initializes its private SIGCHLD handler.
+       * syssignal.h (SA_SIGINFO): Default to 0.
+       * xterm.c (x_term_init): Remove D-bus hack that I installed on May
+       31; it should no longer be needed now.
+
 2013-06-05  Michael Albinus  <michael.albinus@gmx.de>
 
        * emacs.c (main) [HAVE_GFILENOTIFY]: Call globals_of_gfilenotify.

=== modified file 'src/process.c'
--- src/process.c       2013-06-03 18:47:35 +0000
+++ src/process.c       2013-06-05 17:04:13 +0000
@@ -6100,6 +6100,12 @@
    might inadvertently reap a GTK-created process that happened to
    have the same process ID.  */
 
+/* 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.  */
+static void dummy_handler (int sig) {}
+static signal_handler_t volatile lib_child_handler = dummy_handler;
+
 /* Handle a SIGCHLD signal by looking for known child processes of
    Emacs whose status have changed.  For each one found, record its
    new status.
@@ -6184,6 +6190,8 @@
            }
        }
     }
+
+  lib_child_handler (sig);
 }
 
 static void
@@ -7035,9 +7043,13 @@
 void
 catch_child_signal (void)
 {
-  struct sigaction action;
+  struct sigaction action, old_action;
   emacs_sigaction_init (&action, deliver_child_signal);
-  sigaction (SIGCHLD, &action, 0);
+  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;
 }
 
 
@@ -7055,6 +7067,11 @@
   if (! noninteractive || initialized)
 #endif
     {
+#if defined HAVE_GLIB && !defined WINDOWSNT
+      /* Tickle glib's child-handling code so that it initializes its
+        private SIGCHLD handler.  */
+      g_source_unref (g_child_watch_source_new (0));
+#endif
       catch_child_signal ();
     }
 

=== modified file 'src/syssignal.h'
--- src/syssignal.h     2013-01-02 16:13:04 +0000
+++ src/syssignal.h     2013-06-05 17:04:13 +0000
@@ -50,6 +50,10 @@
 # define NSIG NSIG_MINIMUM
 #endif
 
+#ifndef SA_SIGINFO
+# define SA_SIGINFO 0
+#endif
+
 #ifndef emacs_raise
 # define emacs_raise(sig) raise (sig)
 #endif

=== modified file 'src/xterm.c'
--- src/xterm.c 2013-05-31 01:41:52 +0000
+++ src/xterm.c 2013-06-05 17:04:13 +0000
@@ -9897,13 +9897,6 @@
 
         XSetLocaleModifiers ("");
 
-       /* If D-Bus is not already configured, inhibit D-Bus autolaunch,
-          as autolaunch can mess up Emacs's SIGCHLD handler.
-          FIXME: Rewrite subprocess handlers to use glib's child watchers.
-          See Bug#14474.  */
-       if (! egetenv ("DBUS_SESSION_BUS_ADDRESS"))
-         xputenv ("DBUS_SESSION_BUS_ADDRESS=unix:path=/dev/null");
-
         /* Emacs can only handle core input events, so make sure
            Gtk doesn't use Xinput or Xinput2 extensions.  */
        xputenv ("GDK_CORE_DEVICE_EVENTS=1");