[mutter] Fix handling of SIGCHLD

[Dan Winship <danw src gnome org>]

commit 988d2ffab620019573947e3915b5c94ebb225d34
Author: Dan Winship <danw gnome org>
Date:   Thu Nov 19 17:38:06 2009 -0500

    Fix handling of SIGCHLD
    
    The commit that removed metacity-dialog added a global SIGCHLD handler
    that caused problems by (a) calling waitpid(-1) and thus breaking
    g_child_watch for everyone else, and (b) doing too much from a signal
    handler and sometimes causing deadlocks (bug 596200).
    
    This removes the global handler and has each zenity user create its
    own child watch to watch for exit. (It also fixes the window class of
    the zenity dialogs, so that meta_window_present_delete_dialog() will
    work again.)

 src/core/delete.c  |   27 ++++++---------------------
 src/core/main.c    |   41 -----------------------------------------
 src/core/session.c |   31 +++++++++++++++----------------
 src/core/util.c    |   31 +++----------------------------
 src/include/util.h |   30 ------------------------------
 5 files changed, 24 insertions(+), 136 deletions(-)
---
diff --git a/src/core/delete.c b/src/core/delete.c
index 813ae66..20ec1b9 100644
--- a/src/core/delete.c
+++ b/src/core/delete.c
@@ -55,27 +55,16 @@ delete_ping_reply_func (MetaDisplay *display,
   /* we do nothing */
 }
 
-static gboolean
-delete_window_callback (gpointer w_p)
-{
-  meta_window_kill ((MetaWindow*) w_p);
-
-  return FALSE; /* don't do it again */
-}
-
 static void
-sigchld_handler (MetaNexus *nexus, guint arg1, gpointer arg2, gpointer user_data)
+dialog_exited (GPid pid, int status, gpointer user_data)
 {
   MetaWindow *ours = (MetaWindow*) user_data;
 
-  if (GPOINTER_TO_INT (arg2) == ours->dialog_pid)
-    {
-      if (arg1 == 1 /* pressed "force quit" */)
-        g_idle_add_full (G_PRIORITY_DEFAULT,
-                         delete_window_callback, user_data, NULL);
+  ours->dialog_pid = -1;
 
-      ours->dialog_pid = -1; /* forget it anyway */
-    }
+  /* exit status of 1 means the user pressed "Force Quit" */
+  if (WIFEXITED (status) && WEXITSTATUS (status) == 1)
+    meta_window_kill (ours);
 }
 
 static void
@@ -119,11 +108,7 @@ delete_ping_timeout_func (MetaDisplay *display,
   g_free (window_content);
 
   window->dialog_pid = dialog_pid;
-
-  g_signal_connect (sigchld_nexus, "sigchld",
-                    G_CALLBACK (sigchld_handler),
-                    window);
-
+  g_child_watch_add (dialog_pid, dialog_exited, window);
 }
 
 void
diff --git a/src/core/main.c b/src/core/main.c
index 7aa62c9..49b7761 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -444,29 +444,6 @@ sigterm_handler (int signum)
   exit (meta_exit_code);
 }
 
-static guint sigchld_signal_id = 0;
-
-static void
-sigchld_handler (int signum, siginfo_t *info, void *context)
-{
-  int stat;
-
-  if (info->si_code == CLD_EXITED)
-    {
-      g_signal_emit (sigchld_nexus, sigchld_signal_id, 0,
-                     info->si_status,
-                     GINT_TO_POINTER (info->si_pid));
-    }
-
-  g_signal_handlers_disconnect_matched (sigchld_nexus,
-                                        G_SIGNAL_MATCH_DATA,
-                                        sigchld_signal_id,
-                                        0, NULL, NULL,
-                                        GINT_TO_POINTER (info->si_pid));
-
-  waitpid (info->si_pid, &stat, WNOHANG);
-}
-    
 /**
  * This is where the story begins. It parses commandline options and
  * environment variables, sets up the screen, hands control off to
@@ -517,24 +494,6 @@ main (int argc, char **argv)
     g_printerr ("Failed to register SIGTERM handler: %s\n",
 		g_strerror (errno));
 
-  sigchld_nexus = g_object_new (META_TYPE_NEXUS, NULL);
-
-  sigchld_signal_id =
-    g_signal_new ("sigchld", META_TYPE_NEXUS,
-                  G_SIGNAL_RUN_LAST,
-                  0, NULL, NULL,
-                  g_cclosure_marshal_VOID__UINT_POINTER,
-                  G_TYPE_NONE,
-                  2,
-                  G_TYPE_UINT, G_TYPE_POINTER);
-  
-  act.sa_flags = SA_NOCLDSTOP | SA_SIGINFO;
-  act.sa_handler = SIG_DFL;
-  act.sa_sigaction = &sigchld_handler;
-  if (sigaction (SIGCHLD, &act, NULL) < 0)
-    g_printerr ("Failed to register SIGCHLD handler: %s\n",
-		g_strerror (errno));
-
   if (g_getenv ("MUTTER_VERBOSE"))
     meta_set_verbose (TRUE);
   if (g_getenv ("MUTTER_DEBUG"))
diff --git a/src/core/session.c b/src/core/session.c
index 10034ff..34bd8c3 100644
--- a/src/core/session.c
+++ b/src/core/session.c
@@ -29,6 +29,7 @@
 #include <X11/Xatom.h>
 
 #include <time.h>
+#include <sys/wait.h>
 
 #ifndef HAVE_SM
 void
@@ -1749,11 +1750,11 @@ finish_interact (gboolean shutdown)
 }
 
 static void
-sigchld_handler (MetaNexus *nexus, guint arg1, gpointer arg2, gpointer user_data)
+dialog_closed (GPid pid, int status, gpointer user_data)
 {
   gboolean shutdown = GPOINTER_TO_INT (user_data);
 
-  if (arg1 == 0) /* pressed "OK" */
+  if (WIFEXITED (status) && WEXITSTATUS (status) == 0) /* pressed "OK" */
     {
       finish_interact (shutdown);
     }
@@ -1767,6 +1768,7 @@ warn_about_lame_clients_and_finish_interact (gboolean shutdown)
   GSList *lame_details = NULL;
   GSList *tmp;
   GSList *columns = NULL;
+  GPid pid;
   
   windows = meta_display_list_windows (meta_get_display (), META_LIST_DEFAULT);
   tmp = windows;
@@ -1814,23 +1816,20 @@ warn_about_lame_clients_and_finish_interact (gboolean shutdown)
     }
   g_slist_free (lame);
 
-  meta_show_dialog("--list",
-                   _("These windows do not support &quot;save current setup&quot; "
-                     "and will have to be restarted manually next time "
-                     "you log in."),
-                   "240",
-                   meta_screen_get_screen_number (meta_get_display()->active_screen),
-                   NULL, NULL,
-                   None,
-                   columns,
-                   lame_details);
+  pid = meta_show_dialog("--list",
+                         _("These windows do not support &quot;save current setup&quot; "
+                           "and will have to be restarted manually next time "
+                           "you log in."),
+                         "240",
+                         meta_screen_get_screen_number (meta_get_display()->active_screen),
+                         NULL, NULL,
+                         None,
+                         columns,
+                         lame_details);
 
   g_slist_free (lame_details);
 
-  g_signal_connect (sigchld_nexus, "sigchld",
-                    G_CALLBACK (sigchld_handler),
-                    GINT_TO_POINTER (shutdown));
-
+  g_child_watch_add (pid, dialog_closed, GINT_TO_POINTER (shutdown));
 }
 
 #endif /* HAVE_SM */
diff --git a/src/core/util.c b/src/core/util.c
index 53bde09..6a0d508 100644
--- a/src/core/util.c
+++ b/src/core/util.c
@@ -40,8 +40,6 @@
 #include <X11/Xlib.h>   /* must explicitly be included for Solaris; #326746 */
 #include <X11/Xutil.h>  /* Just for the definition of the various gravities */
 
-MetaNexus *sigchld_nexus;
-
 #ifdef HAVE_BACKTRACE
 #include <execinfo.h>
 void
@@ -560,7 +558,7 @@ meta_show_dialog (const char *type,
   int i=0;
   GPid child_pid;
   const char **argvl = g_malloc(sizeof (char*) *
-                                (15 +
+                                (17 +
                                  g_slist_length (columns)*2 +
                                  g_slist_length (entries)));
 
@@ -568,6 +566,8 @@ meta_show_dialog (const char *type,
   argvl[i++] = type;
   argvl[i++] = "--screen";
   argvl[i++] = screen_number_text;
+  argvl[i++] = "--class";
+  argvl[i++] = "mutter-dialog";
   argvl[i++] = "--title";
   /* Translators: This is the title used on dialog boxes */
   argvl[i++] = _("Mutter");
@@ -641,31 +641,6 @@ meta_show_dialog (const char *type,
   return child_pid;
 }
 
-GType
-meta_nexus_get_type (void)
-{
-  static GType nexus_type = 0;
-
-  if (!nexus_type)
-    {
-      static const GTypeInfo nexus_info =
-      {
-        sizeof (MetaNexusClass),
-	NULL, NULL, NULL, NULL, NULL,
-	sizeof (MetaNexus),
-	0,
-	NULL, NULL
-      };
-
-      nexus_type = g_type_register_static (G_TYPE_OBJECT,
-					   "MetaNexus",
-					   &nexus_info,
-					   0);
-    }
-
-  return nexus_type;
-}
-
 /***************************************************************************
  * Later functions: like idles but integrated with the Clutter repaint loop
  ***************************************************************************/
diff --git a/src/include/util.h b/src/include/util.h
index 9d0c5d1..d1f5506 100644
--- a/src/include/util.h
+++ b/src/include/util.h
@@ -131,36 +131,6 @@ GPid meta_show_dialog (const char *type,
 
 #endif /* !WITH_VERBOSE_MODE */
 
-#include <glib-object.h>
-
-#define META_TYPE_NEXUS            (meta_nexus_get_type ())
-#define META_NEXUS(obj)            (G_TYPE_CHECK_INSTANCE_CAST ((obj), META_TYPE_NEXUS, MetaNexus))
-#define META_NEXUS_CLASS(klass)    (G_TYPE_CHECK_CLASS_CAST ((klass), META_TYPE_NEXUS, MetaNexusClass))
-#define META_IS_NEXUS(obj)         (G_TYPE_CHECK_INSTANCE_TYPE ((obj), META_TYPE_NEXUS))
-#define META_IS_NEXUS_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), META_TYPE_NEXUS))
-#define META_NEXUS_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), META_TYPE_NEXUS, MetaNexusClass))
-
-typedef struct _MetaNexus
-{
-  GObject parent_instance;
-} MetaNexus;
-
-typedef struct _MetaNexusClass
-{
-  GObjectClass parent_class;
-} MetaNexusClass;
-
-GType meta_nexus_get_type (void) G_GNUC_CONST;
-MetaNexus *meta_nexus_new ();
-
-/**
- * An object which exists purely to attach signals to; this is to receive
- * signals when a child process exits.  The signal is "sigchld" with no detail.
- *
- * \bug Eventually we should have a specialised type for objects like these.
- */
-extern MetaNexus *sigchld_nexus;
-
 /**
  * MetaLaterType:
  * @META_LATER_RESIZE: call in a resize processing phase that is done