GChildWatch source -- take two



Here's another attempt at the GChildWatch source.  I incorporated most
of the changes proposed, and hope it can now handle the single-threaded
case better.  It now always writes down the pipe and doesn't count on
poll being interrupted by the signal.

Comments?
-Jonathan

Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/glib/ChangeLog,v
retrieving revision 1.1351
diff -u -p -r1.1351 ChangeLog
--- ChangeLog	2 Jan 1997 22:24:13 -0000	1.1351
+++ ChangeLog	17 Jul 2003 20:49:52 -0000
@@ -1,3 +1,16 @@
+Fri Jul  4 00:40:35 2003  Jonathan Blandford  <jrb gnome org>
+
+	* glib/gmain.c (g_child_watch_source_new): Source to handle
+	SIGCHLD messages.  This way multiple libraries can monitor their
+	children's exit status without fighting over who installs the
+	signal handler.
+
+	* glib/gmain.c (g_child_watch_add_full): function to create a
+	GChildWatch source and attach it to a loop.
+
+	* glib/gmain.c (g_child_watch_add): simple function to create a
+	GChildWatch source and attach it to a GMainLoop.
+
 2003-07-12  Matthias Clasen  <maclas gmx de>
 
 	* glib/gprintf.c: Doc additions.
Index: glib/gmain.c
===================================================================
RCS file: /cvs/gnome/glib/glib/gmain.c,v
retrieving revision 1.96
diff -u -p -r1.96 gmain.c
--- glib/gmain.c	9 Jul 2003 23:31:20 -0000	1.96
+++ glib/gmain.c	17 Jul 2003 20:49:52 -0000
@@ -39,7 +39,9 @@
 #include "glib.h"
 #include "gthreadinit.h"
 #include <sys/types.h>
+#include <sys/wait.h>
 #include <time.h>
+#include <fcntl.h>
 #ifdef HAVE_SYS_TIME_H
 #include <sys/time.h>
 #endif /* HAVE_SYS_TIME_H */
@@ -65,6 +67,7 @@
 /* Types */
 
 typedef struct _GTimeoutSource GTimeoutSource;
+typedef struct _GChildWatchSource GChildWatchSource;
 typedef struct _GPollRec GPollRec;
 typedef struct _GSourceCallback GSourceCallback;
 
@@ -157,6 +160,15 @@ struct _GTimeoutSource
   guint       interval;
 };
 
+struct _GChildWatchSource
+{
+  GSource     source;
+  gint        pid;
+  gint        child_status;
+  gint        count;
+  gboolean    child_exited;
+};
+
 struct _GPollRec
 {
   gint priority;
@@ -213,6 +225,12 @@ static gboolean g_timeout_check    (GSou
 static gboolean g_timeout_dispatch (GSource     *source,
 				    GSourceFunc  callback,
 				    gpointer     user_data);
+static gboolean g_child_watch_prepare  (GSource     *source,
+				        gint        *timeout);
+static gboolean g_child_watch_check    (GSource     *source);
+static gboolean g_child_watch_dispatch (GSource     *source,
+					GSourceFunc  callback,
+					gpointer     user_data);
 static gboolean g_idle_prepare     (GSource     *source,
 				    gint        *timeout);
 static gboolean g_idle_check       (GSource     *source);
@@ -221,8 +239,22 @@ static gboolean g_idle_dispatch    (GSou
 				    gpointer     user_data);
 
 G_LOCK_DEFINE_STATIC (main_loop);
+G_LOCK_DEFINE_STATIC (main_context_list);
+
 static GMainContext *default_main_context;
 static GSList *main_contexts_without_pipe = NULL;
+static GMainContext *main_context_about_to_poll = NULL;
+
+/* Child status monitoring code */
+enum {
+  CHILD_WATCH_UNINITIALIZED,
+  CHILD_WATCH_INITIALIZED_SINGLE,
+  CHILD_WATCH_INITIALIZED_THREADED
+};
+static gint child_watch_init_state = CHILD_WATCH_UNINITIALIZED;
+static volatile sig_atomic_t child_watch_count = 1;
+static gint child_watch_wake_up_pipe[2] = {0, 0};
+static GSList *main_context_list = NULL;
 
 #if defined(G_PLATFORM_WIN32) && defined(__GNUC__)
 __declspec(dllexport)
@@ -235,6 +267,14 @@ GSourceFuncs g_timeout_funcs =
   NULL
 };
 
+GSourceFuncs g_child_watch_funcs =
+{
+  g_child_watch_prepare,
+  g_child_watch_check,
+  g_child_watch_dispatch,
+  NULL
+};
+
 #if defined(G_PLATFORM_WIN32) && defined(__GNUC__)
 __declspec(dllexport)
 #endif
@@ -561,6 +601,10 @@ g_main_context_unref_and_unlock (GMainCo
       return;
     }
 
+  G_LOCK (main_context_list);
+  main_context_list = g_slist_remove (main_context_list, context);
+  G_UNLOCK (main_context_list);
+
   source = context->source_list;
   while (source)
     {
@@ -615,7 +659,6 @@ g_main_context_unref (GMainContext *cont
   g_main_context_unref_and_unlock (context);
 }
 
-#ifdef G_THREADS_ENABLED
 static void 
 g_main_context_init_pipe (GMainContext *context)
 {
@@ -640,8 +683,9 @@ g_main_context_init_pipe (GMainContext *
   g_main_context_add_poll_unlocked (context, 0, &context->wake_up_rec);
 }
 
-void
-_g_main_thread_init ()
+/* called when initializing threads and the child_watch source */
+static void
+add_pipes_to_main_contexts (void)
 {
   GSList *curr = main_contexts_without_pipe;
   while (curr)
@@ -652,6 +696,14 @@ _g_main_thread_init ()
   g_slist_free (main_contexts_without_pipe);
   main_contexts_without_pipe = NULL;  
 }
+
+#ifdef G_THREADS_ENABLED
+
+void
+_g_main_thread_init ()
+{
+  add_pipes_to_main_contexts ();
+}
 #endif /* G_THREADS_ENABLED */
 
 /**
@@ -700,7 +752,11 @@ g_main_context_new ()
 						  context);
 #endif
 
-  return context;
+  G_LOCK (main_context_list);
+  main_context_list = g_slist_append (main_context_list, context);
+  G_UNLOCK (main_context_list);
+
+ return context;
 }
 
 /**
@@ -1513,11 +1569,11 @@ g_main_context_find_source_by_user_data 
  * g_source_remove:
  * @tag: the id of the source to remove.
  * 
- * Removes the source with the given id from the default main
- * context. The id of a #GSource is given by g_source_get_id(),
- * or will be returned by the functions g_source_attach(),
- * g_idle_add(), g_idle_add_full(), g_timeout_add(),
- * g_timeout_add_full(), g_io_add_watch, and g_io_add_watch_full().
+ * Removes the source with the given id from the default main context. The id of
+ * a #GSource is given by g_source_get_id(), or will be returned by the
+ * functions g_source_attach(), g_idle_add(), g_idle_add_full(),
+ * g_timeout_add(), g_timeout_add_full(), g_child_watch_add(),
+ * g_child_watch_add_full(), g_io_add_watch(), and g_io_add_watch_full().
  *
  * See also g_source_destroy().
  *
@@ -2173,8 +2229,8 @@ g_main_context_check (GMainContext *cont
   if (!context->poll_waiting)
     {
 #ifndef G_OS_WIN32
-      gchar c;
-      read (context->wake_up_pipe[0], &c, 1);
+      gchar a;
+      read (context->wake_up_pipe[0], &a, 1);
 #endif
     }
   else
@@ -2328,6 +2384,16 @@ g_main_context_iterate (GMainContext *co
   
   UNLOCK_CONTEXT (context);
 
+  /* main_context_about_to_poll is entirely for the SIGCHLD source.  We normally
+   * count on SIGCHLD waking up the poll when it occurs during that time.
+   * However, if the SIGCHLD occurs between prepare and poll, we can miss it and
+   * block indefinitely, waiting for the child to exit.  To avoid this, we store
+   * the context in main_context_about_to_poll in the expectation that the
+   * signal handler can deal with it.  We expect that no code in prepare or
+   * query tries to send data to the wake up pipe, and that we can safely write
+   * a byte down it. */
+
+  main_context_about_to_poll = context;
   some_ready = g_main_context_prepare (context, &max_priority); 
   
   while ((nfds = g_main_context_query (context, max_priority, &timeout, fds, 
@@ -2344,7 +2410,8 @@ g_main_context_iterate (GMainContext *co
     timeout = 0;
   
   g_main_context_poll (context, timeout, max_priority, fds, nfds);
-  
+  main_context_about_to_poll = NULL;
+
   g_main_context_check (context, max_priority, fds, nfds);
   
   if (dispatch)
@@ -3211,6 +3278,299 @@ g_timeout_add (guint32        interval,
 			     interval, function, data, NULL);
 }
 
+/* Child watch functions */
+
+static void
+check_for_child_exited (GSource *source)
+{
+  GChildWatchSource *child_watch_source;
+  volatile gint count;
+
+  /* protect against another SIGCHLD in the middle of this call */
+  count = child_watch_count;
+
+  child_watch_source = (GChildWatchSource *) source;
+
+  if (child_watch_source->count < count)
+    {
+      gint child_status;
+
+      if (waitpid (child_watch_source->pid, &child_status, WNOHANG) > 0)
+	{
+	  child_watch_source->child_status = child_status;
+	  child_watch_source->child_exited = TRUE;
+	}
+
+      child_watch_source->count = count;
+    }
+}
+
+static gboolean
+g_child_watch_prepare (GSource *source,
+		       gint    *timeout)
+{
+  GChildWatchSource *child_watch_source;
+  *timeout = -1;
+
+  child_watch_source = (GChildWatchSource *) source;
+
+  check_for_child_exited (source);
+  return child_watch_source->child_exited;
+}
+
+
+static gboolean 
+g_child_watch_check (GSource  *source)
+{
+  GChildWatchSource *child_watch_source;
+
+  child_watch_source = (GChildWatchSource *) source;
+
+  return child_watch_source->child_exited;
+}
+
+static gboolean
+g_child_watch_dispatch (GSource    *source, 
+			GSourceFunc callback,
+			gpointer    user_data)
+{
+  GChildWatchSource *child_watch_source;
+  GChildWatchFunc child_watch_callback = (GChildWatchFunc) callback;
+
+  child_watch_source = (GChildWatchSource *) source;
+
+  if (!callback)
+    {
+      g_warning ("Child watch source dispatched without callback\n"
+		 "You must call g_source_set_callback().");
+      return FALSE;
+    }
+
+  (child_watch_callback) (child_watch_source->pid, child_watch_source->child_status, user_data);
+
+  /* We never keep a child watch source around as the child is gone */
+  return FALSE;
+}
+
+static void
+g_child_watch_signal_handler (int signum)
+{
+  child_watch_count ++;
+  /* guard against us modifying errno during the write */
+  int old_errno = errno;
+
+  if (child_watch_init_state == CHILD_WATCH_INITIALIZED_THREADED)
+    {
+      write (child_watch_wake_up_pipe[1], "B", 1);
+    }
+  else
+    {
+      if (main_context_about_to_poll)
+	{
+	  g_main_context_wakeup_unlocked (main_context_about_to_poll);
+	}
+    }
+  errno = old_errno;
+}
+ 
+static void
+g_child_watch_set_up_signal_handler (void)
+{
+  sigset_t newset;
+
+  signal (SIGCHLD, g_child_watch_signal_handler);
+
+  sigemptyset (&newset);
+  sigaddset (&newset, SIGCHLD);
+  sigprocmask (SIG_UNBLOCK, &newset, NULL);
+}
+
+static void
+g_child_watch_source_init_single (void)
+{
+  g_assert (! g_thread_supported());
+  g_assert (child_watch_init_state == CHILD_WATCH_UNINITIALIZED);
+
+  add_pipes_to_main_contexts ();
+  
+  child_watch_init_state = CHILD_WATCH_INITIALIZED_SINGLE;
+
+  g_child_watch_set_up_signal_handler ();
+}
+
+/* Separate thread that sits around waiting for messages from the signal
+ * handler.
+ */
+static gpointer
+child_watch_helper_thread (gpointer data)
+{
+  while (1)
+    {
+      gchar b[20];
+      GSList *list;
+
+      read (child_watch_wake_up_pipe[0], b, 20);
+
+      /* We were woken up.  Wake up all other contexts in all other threads */
+      G_LOCK (main_context_list);
+      for (list = main_context_list; list; list = list->next)
+	{
+	  GMainContext *context;
+
+	  context = list->data;
+	  g_main_context_wakeup (context);
+	}
+      G_UNLOCK (main_context_list);
+    }
+  return NULL;
+}
+
+#ifdef G_THREADS_ENABLED
+
+static void
+g_child_watch_source_init_multi_threaded (void)
+{
+  GError *error = NULL;
+
+  g_assert (g_thread_supported());
+
+  if (pipe (child_watch_wake_up_pipe) < 0)
+    g_error ("Cannot create wake up pipe: %s\n", g_strerror (errno));
+  fcntl (child_watch_wake_up_pipe[1], F_SETFL, O_NONBLOCK | fcntl (child_watch_wake_up_pipe[1], F_GETFL));
+
+  /* We create a helper thread that polls on the wakeup pipe indefinitely */
+  /* FIXME: Think this through for races */
+  if (g_thread_create_full (child_watch_helper_thread, NULL, 0, FALSE, FALSE, G_THREAD_PRIORITY_NORMAL, &error) == NULL)
+    g_error ("Cannot create a thread to monitor child exit status: %s\n", error->message);
+  child_watch_init_state = CHILD_WATCH_INITIALIZED_THREADED;
+
+  g_child_watch_set_up_signal_handler ();
+}
+
+static void
+g_child_watch_source_init_promote_single_to_threaded (void)
+{
+  g_child_watch_source_init_multi_threaded ();
+}
+
+#endif
+
+static void
+g_child_watch_source_init (void)
+{
+#ifdef G_THREADS_ENABLED
+  if (g_thread_supported())
+    {
+      if (child_watch_init_state == CHILD_WATCH_UNINITIALIZED)
+	g_child_watch_source_init_multi_threaded ();
+      else if (child_watch_init_state == CHILD_WATCH_INITIALIZED_SINGLE)
+	g_child_watch_source_init_promote_single_to_threaded ();
+    }
+  else
+    {
+#endif
+      if (child_watch_init_state == CHILD_WATCH_UNINITIALIZED)
+	g_child_watch_source_init_single ();
+#ifdef G_THREADS_ENABLED
+    }
+#endif
+}
+
+/**
+ * g_child_watch_source_new:
+ * @pid: process id of a child process to watch
+ * 
+ * Creates a new child_watch source.  This source is used to monitor a child
+ * process.  It will be triggered once when a SIGCHLD occurs, and the process
+ * specified by pid has exited.  Any application using this source should not
+ * modify the SIGCHILD signal handler.  Instead, they should use a child watch
+ * source to keep track of the exit status of any children that they care about.
+ * Additionally, as glib installs a SIGCHLD signal handler, system calls can be
+ * interrupted and should probably be checked for EINTR.
+ *
+ * The source will not initially be associated with any #GMainContext and must
+ * be added to one with g_source_attach() before it will be executed.  Unlike
+ * other sources, any callback associated with it will only be called at most
+ * once and no return value is expected.
+ * 
+ * Return value: the newly-created child watch source
+ **/
+GSource *
+g_child_watch_source_new (gint pid)
+{
+  GSource *source = g_source_new (&g_child_watch_funcs, sizeof (GChildWatchSource));
+  GChildWatchSource *child_watch_source = (GChildWatchSource *)source;
+
+  g_child_watch_source_init ();
+
+  child_watch_source->pid = pid;
+  /* We want to run on the first pass no matter what, to catch the case where
+   * the child has exited before the signal handler has been installed. */
+  child_watch_source->count = child_watch_count - 1;
+
+  return source;
+}
+
+/**
+ * g_child_watch_add_full:
+ * @priority: the priority of the idle source. Typically this will be in the
+ *            range between #G_PRIORITY_DEFAULT_IDLE and #G_PRIORITY_HIGH_IDLE.
+ * @pid:      process id of a child process to watch
+ * @function: function to call
+ * @data:     data to pass to @function
+ * @notify:   function to call when the idle is removed, or %NULL
+ * 
+ * Sets a function to be called when the child indicated by pid exits, at a
+ * default priority, #G_PRIORITY_DEFAULT.  See g_child_watch_source_new() for
+ * additional comments regarding this source.
+ * 
+ * Return value: the id of event source.
+ **/
+guint
+g_child_watch_add_full (gint            priority,
+			gint            pid,
+			GChildWatchFunc function,
+			gpointer        data,
+			GDestroyNotify  notify)
+{
+  GSource *source;
+  guint id;
+  
+  g_return_val_if_fail (function != NULL, 0);
+
+  source = g_child_watch_source_new (pid);
+
+  if (priority != G_PRIORITY_DEFAULT)
+    g_source_set_priority (source, priority);
+
+  g_source_set_callback (source, (GSourceFunc) function, data, notify);
+  id = g_source_attach (source, NULL);
+  g_source_unref (source);
+
+  return id;
+}
+
+/**
+ * g_child_watch_add:
+ * @pid:      process id of a child process to watch
+ * @function: function to call
+ * @data:     data to pass to @function
+ * 
+ * Sets a function to be called when the child indicated by pid exits, at a
+ * default priority, #G_PRIORITY_DEFAULT.  See g_child_watch_source_new() for
+ * additional comments regarding this source.
+ * 
+ * Return value: the id of event source.
+ **/
+guint 
+g_child_watch_add (gint            pid,
+		   GChildWatchFunc function,
+		   gpointer        data)
+{
+  return g_child_watch_add_full (G_PRIORITY_DEFAULT, pid, function, data, NULL);
+}
+
+ 
 /* Idle functions */
 
 static gboolean 
Index: glib/gmain.h
===================================================================
RCS file: /cvs/gnome/glib/glib/gmain.h,v
retrieving revision 1.16
diff -u -p -r1.16 gmain.h
--- glib/gmain.h	5 Jun 2003 22:18:27 -0000	1.16
+++ glib/gmain.h	17 Jul 2003 20:49:52 -0000
@@ -32,6 +32,9 @@ typedef struct _GSourceCallbackFuncs	GSo
 typedef struct _GSourceFuncs	        GSourceFuncs;
 
 typedef gboolean (*GSourceFunc)       (gpointer data);
+typedef void     (*GChildWatchFunc)   (gint     pid,
+				       gint     status,
+				       gpointer data);
 
 struct _GSource
 {
@@ -243,8 +246,9 @@ void     g_source_get_current_time (GSou
 
 /* Specific source types
  */
-GSource *g_idle_source_new    (void);
-GSource *g_timeout_source_new (guint         interval);
+GSource *g_idle_source_new        (void);
+GSource *g_child_watch_source_new (gint  pid);
+GSource *g_timeout_source_new     (guint interval);
 
 /* Miscellaneous functions
  */
@@ -278,25 +282,34 @@ gboolean g_source_remove_by_user_data   
 gboolean g_source_remove_by_funcs_user_data  (GSourceFuncs  *funcs,
 					      gpointer       user_data);
 
-/* Idles and timeouts */
-guint		g_timeout_add_full	(gint           priority,
-					 guint          interval, 
-					 GSourceFunc    function,
-					 gpointer       data,
-					 GDestroyNotify notify);
-guint		g_timeout_add		(guint          interval,
-					 GSourceFunc    function,
-					 gpointer       data);
-guint		g_idle_add	   	(GSourceFunc	function,
-					 gpointer	data);
-guint	   	g_idle_add_full		(gint   	priority,
-					 GSourceFunc	function,
-					 gpointer	data,
-					 GDestroyNotify notify);
-gboolean	g_idle_remove_by_data	(gpointer	data);
+/* Idles, child watchers and timeouts */
+guint    g_timeout_add_full     (gint            priority,
+				 guint           interval,
+				 GSourceFunc     function,
+				 gpointer        data,
+				 GDestroyNotify  notify);
+guint    g_timeout_add          (guint           interval,
+				 GSourceFunc     function,
+				 gpointer        data);
+guint    g_child_watch_add_full (gint            priority,
+				 gint            pid,
+				 GChildWatchFunc function,
+				 gpointer        data,
+				 GDestroyNotify  notify);
+guint    g_child_watch_add      (gint            pid,
+				 GChildWatchFunc function,
+				 gpointer        data);
+guint    g_idle_add             (GSourceFunc     function,
+				 gpointer        data);
+guint    g_idle_add_full        (gint            priority,
+				 GSourceFunc     function,
+				 gpointer        data,
+				 GDestroyNotify  notify);
+gboolean g_idle_remove_by_data  (gpointer        data);
 
 /* Hook for GClosure / GSource integration. Don't touch */
 GLIB_VAR GSourceFuncs g_timeout_funcs;
+GLIB_VAR GSourceFuncs g_child_watch_funcs;
 GLIB_VAR GSourceFuncs g_idle_funcs;
 
 G_END_DECLS
Index: tests/Makefile.am
===================================================================
RCS file: /cvs/gnome/glib/tests/Makefile.am,v
retrieving revision 1.56
diff -u -p -r1.56 Makefile.am
--- tests/Makefile.am	27 May 2003 22:12:40 -0000	1.56
+++ tests/Makefile.am	17 Jul 2003 20:49:52 -0000
@@ -64,6 +64,7 @@ endif
 test_programs =					\
 	array-test				\
 	$(CXX_TEST)				\
+	child-test				\
 	date-test				\
 	dirname-test				\
 	gio-test				\
@@ -109,6 +110,7 @@ thread_ldadd = $(libgthread) $(G_THREAD_
 module_ldadd = $(libgmodule) $(G_MODULE_LIBS) $(progs_ldadd)
 
 array_test_LDADD = $(progs_ldadd)
+child_test_LDADD = $(thread_ldadd) 
 date_test_LDADD = $(progs_ldadd)
 dirname_test_LDADD = $(progs_ldadd)
 gio_test_LDADD = $(progs_ldadd)
Index: tests/child-test.c
===================================================================
RCS file: tests/child-test.c
diff -N tests/child-test.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ tests/child-test.c	17 Jul 2003 20:49:52 -0000
@@ -0,0 +1,69 @@
+#include <sys/types.h>
+#include <unistd.h>
+#include "glib.h"
+#include <stdlib.h>
+
+GMainLoop *main_loop;
+
+gint
+get_a_child (void)
+{
+  gint pid;
+
+  pid = fork ();
+  if (pid < 0)
+    exit (1);
+
+  if (pid > 0)
+    return pid;
+
+  sleep (1);
+  _exit (0);
+}
+
+gint
+child_watch_callback (gint pid, gint status, gpointer data)
+{
+  g_print ("child %d exited, status %d\n", pid, status);
+}
+
+static gpointer
+test_thread (gpointer data)
+{
+  GMainLoop *new_main_loop;
+  GSource *source;
+  gint pid;
+
+  new_main_loop = g_main_loop_new (NULL, FALSE);
+
+  pid = get_a_child ();
+  source = g_child_watch_source_new (pid);
+  g_source_set_callback (source, (GSourceFunc) child_watch_callback, NULL, NULL);
+  g_source_attach (source, g_main_loop_get_context (new_main_loop));
+  g_source_unref (source);
+
+  g_print ("whee! created pid: %d\n", pid);
+  g_main_loop_run (new_main_loop);
+
+}
+
+int
+main (int argc, char *argv[])
+{
+  g_thread_init (NULL);
+  main_loop = g_main_loop_new (NULL, FALSE);
+
+
+  system ("/bin/true");
+#if 0
+  pid = get_a_child ();
+  g_child_watch_add (pid, child_watch_callback, NULL);
+#endif  
+  g_thread_create (test_thread, NULL, FALSE, NULL);
+
+  g_thread_create (test_thread, NULL, FALSE, NULL);
+
+  g_main_loop_run (main_loop);
+
+  return 0;
+}


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]