GNOME.org
 

[glib/wip/child-catchall] g_unix_catchall_child_source_new: New API to do waitpid(-1, ...)

commit c5d32c4098a428f7dce14b017442c13b1bb39117
Author: Colin Walters <walters verbum org>
Date:   Thu Oct 25 02:54:05 2012 -0400

    g_unix_catchall_child_source_new: New API to do waitpid(-1, ...)
    
    [STILL EXPERIMENTAL]
    
    With the goal of modifying gnome-session to use the new Linux
    PR_SET_CHILD_SUBREAPER, it needs to be able to reap arbitary children
    that may be reparented to it.
    
    But if gnome-session is going to continue to work with GLib and GIO,
    even if we converted all child watches in gnome-session to use this
    new API, we'd still have to contend with the possibility of it having
    a process spawned indirectly (stuff like the GDBus dbus-launch
    invocation).
    
    There is the separate but related issue of continuing to support
    spawning processes in multiple threads.  gnome-session should be able
    to call g_spawn_sync() from a helper thread, without its waitpid()
    invocation racing with the waitpid(-1, ...) from the GLib worker
    thread.
    
    In order to make that case work, explicitly block the waitpid(-1, )
    while a g_spawn_sync() is in progress.  This isn't ideal, but it's
    better than being racy.

 glib/glib-unix.h      |    3 +
 glib/gmain-internal.h |    3 +
 glib/gmain.c          |  200 ++++++++++++++++++++++++++++++++++++++++++++++---
 glib/gspawn.c         |   10 ++-
 glib/tests/unix.c     |   52 +++++++++++++
 5 files changed, 255 insertions(+), 13 deletions(-)
---
diff --git a/glib/glib-unix.h b/glib/glib-unix.h
index c04f66f..0b2e3d2 100644
--- a/glib/glib-unix.h
+++ b/glib/glib-unix.h
@@ -85,6 +85,9 @@ guint    g_unix_signal_add         (gint        signum,
                                     GSourceFunc handler,
                                     gpointer    user_data);
 
+GLIB_AVAILABLE_IN_2_36
+GSource *g_unix_catchall_child_source_new (void);
+
 G_END_DECLS
 
 #endif  /* __G_UNIX_H__ */
diff --git a/glib/gmain-internal.h b/glib/gmain-internal.h
index 648aff3..ce799c6 100644
--- a/glib/gmain-internal.h
+++ b/glib/gmain-internal.h
@@ -30,6 +30,9 @@ G_BEGIN_DECLS
 
 GSource *_g_main_create_unix_signal_watch (int signum);
 
+void _g_main_disable_catchall_child_watch (void);
+void _g_main_enable_catchall_child_watch (void);
+
 G_END_DECLS
 
 #endif /* __G_MAIN_H__ */
diff --git a/glib/gmain.c b/glib/gmain.c
index c0c4581..d15b811 100644
--- a/glib/gmain.c
+++ b/glib/gmain.c
@@ -185,6 +185,7 @@
 typedef struct _GTimeoutSource GTimeoutSource;
 typedef struct _GChildWatchSource GChildWatchSource;
 typedef struct _GUnixSignalWatchSource GUnixSignalWatchSource;
+typedef struct _GUnixCatchallChildWatchSource GUnixCatchallChildWatchSource;
 typedef struct _GPollRec GPollRec;
 typedef struct _GSourceCallback GSourceCallback;
 
@@ -303,6 +304,12 @@ struct _GUnixSignalWatchSource
   gboolean    pending;
 };
 
+struct _GUnixCatchallChildWatchSource
+{
+  GSource     source;
+  GSList     *pending_waits;
+};
+
 struct _GPollRec
 {
   GPollFD *fd;
@@ -395,6 +402,13 @@ static gboolean g_unix_signal_watch_dispatch (GSource     *source,
 					      GSourceFunc  callback,
 					      gpointer     user_data);
 static void     g_unix_signal_watch_finalize  (GSource     *source);
+static gboolean g_unix_catchall_watch_prepare  (GSource     *source,
+					      gint        *timeout);
+static gboolean g_unix_catchall_watch_check    (GSource     *source);
+static gboolean g_unix_catchall_watch_dispatch (GSource     *source,
+					      GSourceFunc  callback,
+					      gpointer     user_data);
+static void     g_unix_catchall_watch_finalize  (GSource     *source);
 #endif
 static gboolean g_idle_prepare     (GSource     *source,
 				    gint        *timeout);
@@ -428,6 +442,15 @@ static volatile int any_unix_signal_pending;
 G_LOCK_DEFINE_STATIC (unix_signal_lock);
 static GSList *unix_signal_watches;
 static GSList *unix_child_watches;
+static GSList *unix_catchall_child_watches;
+
+typedef struct {
+  pid_t pid;
+  gint estatus;
+  volatile gint refcount; /* atomic */
+} UnixWaitStatus;
+
+static volatile gint unix_catchall_disable_counter; /* Access via g_atomic_int */
 
 static GSourceFuncs g_unix_signal_funcs =
 {
@@ -436,6 +459,14 @@ static GSourceFuncs g_unix_signal_funcs =
   g_unix_signal_watch_dispatch,
   g_unix_signal_watch_finalize
 };
+
+static GSourceFuncs g_unix_catchall_funcs =
+{
+  g_unix_catchall_watch_prepare,
+  g_unix_catchall_watch_check,
+  g_unix_catchall_watch_dispatch,
+  g_unix_catchall_watch_finalize
+};
 #endif /* !G_OS_WIN32 */
 G_LOCK_DEFINE_STATIC (main_context_list);
 static GSList *main_context_list = NULL;
@@ -4405,15 +4436,19 @@ dispatch_unix_signals (void)
   /* handle GChildWatchSource instances */
   if (unix_signal_pending[SIGCHLD])
     {
+      int estatus;
+      pid_t pid;
+
       unix_signal_pending[SIGCHLD] = FALSE;
 
       /* The only way we can do this is to scan all of the children.
        *
-       * The docs promise that we will not reap children that we are not
-       * explicitly watching, so that ties our hands from calling
-       * waitpid(-1).  We also can't use siginfo's si_pid field since if
-       * multiple SIGCHLD arrive at the same time, one of them can be
-       * dropped (since a given UNIX signal can only be pending once).
+       * The docs promise that we will not reap children that we are
+       * not explicitly watching, so that ties our hands from calling
+       * waitpid(-1) - that is a separate _catchall API.  We also
+       * can't use siginfo's si_pid field since if multiple SIGCHLD
+       * arrive at the same time, one of them can be dropped (since a
+       * given UNIX signal can only be pending once).
        */
       for (node = unix_child_watches; node; node = node->next)
         {
@@ -4429,6 +4464,40 @@ dispatch_unix_signals (void)
                 }
             }
         }
+
+      /* Now see if we have at least one catchall watch, and it isn't
+       * currently blocked internally.  If both are true, we can
+       * freely call waitpid(-1).
+       */
+      if (unix_catchall_child_watches &&
+          g_atomic_int_get (&unix_catchall_disable_counter) == 0)
+        {
+          gboolean caught_one = FALSE;
+
+          while ((pid = waitpid (-1, &estatus, WNOHANG)) > 0)
+            {
+              caught_one = TRUE;
+
+              for (node = unix_catchall_child_watches; node; node = node->next)
+                {
+                  GUnixCatchallChildWatchSource *source = node->data;
+                  UnixWaitStatus *waitstatus = g_slice_new (UnixWaitStatus);
+                  waitstatus->pid = pid;
+                  waitstatus->estatus = estatus;
+                  
+                  source->pending_waits = g_slist_prepend (source->pending_waits, waitstatus);
+                }
+            }
+
+          if (caught_one)
+            {
+              for (node = unix_catchall_child_watches; node; node = node->next)
+                {
+                  GUnixCatchallChildWatchSource *source = node->data;
+                  wake_source ((GSource *) source);
+                }
+            }
+        }
     }
 
   /* handle GUnixSignalWatchSource instances */
@@ -4472,6 +4541,14 @@ g_child_watch_check (GSource *source)
   return child_watch_source->child_exited;
 }
 
+static void
+g_child_watch_finalize (GSource *source)
+{
+  G_LOCK (unix_signal_lock);
+  unix_child_watches = g_slist_remove (unix_child_watches, source);
+  G_UNLOCK (unix_signal_lock);
+}
+
 static gboolean
 g_unix_signal_watch_prepare (GSource *source,
 			     gint    *timeout)
@@ -4518,6 +4595,14 @@ g_unix_signal_watch_dispatch (GSource    *source,
 }
 
 static void
+g_unix_signal_watch_finalize (GSource    *source)
+{
+  G_LOCK (unix_signal_lock);
+  unix_signal_watches = g_slist_remove (unix_signal_watches, source);
+  G_UNLOCK (unix_signal_lock);
+}
+
+static void
 ensure_unix_signal_handler_installed_unlocked (int signum)
 {
   static sigset_t installed_signal_mask;
@@ -4565,23 +4650,116 @@ _g_main_create_unix_signal_watch (int signum)
   return source;
 }
 
+static gboolean
+g_unix_catchall_watch_prepare (GSource *source,
+                               gint    *timeout)
+{
+  GUnixCatchallChildWatchSource *catchall_source;
+
+  catchall_source = (GUnixCatchallChildWatchSource *) source;
+
+  return catchall_source->pending_waits != NULL;
+}
+
+static gboolean
+g_unix_catchall_watch_check (GSource  *source)
+{
+  GUnixCatchallChildWatchSource *catchall_source;
+
+  catchall_source = (GUnixCatchallChildWatchSource *) source;
+
+  return catchall_source->pending_waits != NULL;
+}
+
+static gboolean
+g_unix_catchall_watch_dispatch (GSource    *source, 
+                                GSourceFunc callback,
+                                gpointer    user_data)
+{
+  GUnixCatchallChildWatchSource *catchall_source;
+  GChildWatchFunc child_callback = (GChildWatchFunc) callback;
+  GSList *iter;
+
+  catchall_source = (GUnixCatchallChildWatchSource *) source;
+
+  if (!callback)
+    {
+      g_warning ("Unix catchall source dispatched without callback\n"
+		 "You must call g_source_set_callback().");
+      return FALSE;
+    }
+
+  for (iter = catchall_source->pending_waits; iter; iter = iter->next)
+    {
+      UnixWaitStatus *status = iter->data;
+
+      (child_callback) (status->pid, status->estatus, user_data);
+      
+      g_slice_free (UnixWaitStatus, status);
+    }
+
+  g_clear_pointer (&catchall_source->pending_waits, (GDestroyNotify) g_slist_free);
+
+  return TRUE;
+}
+
 static void
-g_unix_signal_watch_finalize (GSource    *source)
+g_unix_catchall_watch_finalize (GSource *source)
 {
   G_LOCK (unix_signal_lock);
-  unix_signal_watches = g_slist_remove (unix_signal_watches, source);
+  unix_catchall_child_watches = g_slist_remove (unix_catchall_child_watches, source);
   G_UNLOCK (unix_signal_lock);
 }
 
-static void
-g_child_watch_finalize (GSource *source)
+/**
+ * g_unix_catchall_child_source_new:
+ *
+ * Normally, g_child_watch_add() is used to monitor child processes.
+ * This function exists to support very unusual circumstances, such as
+ * writing a process that behaves like a traditional Unix "init"
+ * program, where the operating system kernel will cause "grandchild"
+ * processes to be reparented if their parent terminates.
+ *
+ * In other words, this function provides the equivalent of
+ * <literal>waitpid(-1, ...)</literal>.  However, the source is only
+ * triggered for child PIDs which do not already have an active child
+ * watch.
+ *
+ * On Linux, it makes sense to use this together with
+ * <literal>prctl(PR_SET_CHILD_SUBREAPER)</literal>.
+ *
+ * Returns: A new #GSource
+ *
+ * Since: 2.36
+ */
+GSource *
+g_unix_catchall_child_source_new (void)
 {
+  GSource *source;
+
+  source = g_source_new (&g_unix_catchall_funcs, sizeof (GUnixCatchallChildWatchSource));
+
   G_LOCK (unix_signal_lock);
-  unix_child_watches = g_slist_remove (unix_child_watches, source);
+  ensure_unix_signal_handler_installed_unlocked (SIGCHLD);
+  unix_catchall_child_watches = g_slist_prepend (unix_catchall_child_watches, source);
   G_UNLOCK (unix_signal_lock);
+
+  return source;
 }
 
-#endif /* G_OS_WIN32 */
+void
+_g_main_disable_catchall_child_watch (void)
+{
+  g_atomic_int_inc (&unix_catchall_disable_counter);
+}
+
+void
+_g_main_enable_catchall_child_watch (void)
+{
+  g_atomic_int_add (&unix_catchall_disable_counter, -1);
+}
+
+#endif /* !G_OS_WIN32 */
 
 static gboolean
 g_child_watch_dispatch (GSource    *source, 
diff --git a/glib/gspawn.c b/glib/gspawn.c
index 14d1b4c..4d6925c 100644
--- a/glib/gspawn.c
+++ b/glib/gspawn.c
@@ -48,6 +48,7 @@
 #include "gmem.h"
 #include "gshell.h"
 #include "gstring.h"
+#include "gmain-internal.h"
 #include "gstrfuncs.h"
 #include "gtestutils.h"
 #include "gutils.h"
@@ -265,7 +266,7 @@ g_spawn_sync (const gchar          *working_directory,
   gint ret;
   GString *outstr = NULL;
   GString *errstr = NULL;
-  gboolean failed;
+  gboolean failed = TRUE;
   gint status;
   
   g_return_val_if_fail (argv != NULL, FALSE);
@@ -283,6 +284,8 @@ g_spawn_sync (const gchar          *working_directory,
 
   if (standard_error)
     *standard_error = NULL;
+
+  _g_main_disable_catchall_child_watch ();
   
   if (!fork_exec_with_pipes (FALSE,
                              working_directory,
@@ -302,7 +305,7 @@ g_spawn_sync (const gchar          *working_directory,
                              standard_output ? &outpipe : NULL,
                              standard_error ? &errpipe : NULL,
                              error))
-    return FALSE;
+    goto out;
 
   /* Read data from child. */
   
@@ -438,6 +441,9 @@ g_spawn_sync (const gchar          *working_directory,
             }
         }
     }
+
+ out:
+  _g_main_enable_catchall_child_watch ();
   
   if (failed)
     {
diff --git a/glib/tests/unix.c b/glib/tests/unix.c
index 329e19a..b41f091 100644
--- a/glib/tests/unix.c
+++ b/glib/tests/unix.c
@@ -147,6 +147,57 @@ test_sighup_add_remove (void)
 
 }
 
+typedef struct {
+  GPid pid;
+  GMainLoop *loop;
+} CatchAllData;
+
+static void
+on_catchall_child (GPid   pid,
+                   gint   estatus,
+                   gpointer user_data)
+{
+  CatchAllData *data = user_data;
+  GError *local_error = NULL;
+  GError **error = &local_error;
+
+  g_spawn_check_exit_status (estatus, error);
+  g_assert_no_error (local_error);
+
+  if (pid == data->pid)
+    g_main_loop_quit (data->loop);
+}
+
+static void
+test_catchall_source (void)
+{
+  GMainLoop *mainloop;
+  CatchAllData data;
+  GSource *source;
+  GPid pid;
+  GError *local_error = NULL;
+  GError **error = &local_error;
+  char *child_args[] = { "/bin/true", "/bin/true", NULL };
+
+  memset (&data, 0, sizeof (data));
+  mainloop = g_main_loop_new (NULL, FALSE);
+  data.loop = mainloop;
+
+  source = g_unix_catchall_child_source_new ();
+  g_source_set_callback (source, (GSourceFunc)on_catchall_child, &data, NULL);
+  g_source_attach (source, NULL);
+  g_source_unref (source);
+
+  g_spawn_async (NULL, (char**)child_args, NULL, G_SPAWN_DO_NOT_REAP_CHILD,
+                 NULL, NULL, &pid, error);
+  g_assert_no_error (local_error);
+  data.pid = pid;
+
+  g_main_loop_run (mainloop);
+
+  g_main_loop_unref (mainloop);
+}
+
 int
 main (int   argc,
       char *argv[])
@@ -159,6 +210,7 @@ main (int   argc,
   g_test_add_func ("/glib-unix/sigterm", test_sigterm);
   g_test_add_func ("/glib-unix/sighup_again", test_sighup);
   g_test_add_func ("/glib-unix/sighup_add_remove", test_sighup_add_remove);
+  g_test_add_func ("/glib-unix/catchall-source", test_catchall_source);
 
   return g_test_run();
 }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]