[glib/wip/gcleanup] gsubprocess: Cleanup GSubprocess correctly

[Stefan Walter <stefw src gnome org>]

commit e96aeb7a68506fc2694ed4ee36b824ea8c5ffb8e
Author: Stef Walter <stefw gnome org>
Date:   Sat Nov 9 08:55:17 2013 +0100

    gsubprocess: Cleanup GSubprocess correctly
    
    Although it's nice that GSubprocess object lives until its
    child goes away, this can't be the case during cleanup and or
    unloading of gio, since the GLib worker thread will run non-existant
    code.
    
    So in the case that GIO hits cleanup, we finalize the GSubprocess
    regardless of the state of its child.

 gio/gsubprocess.c       |   51 +++++++++++++++++++++++++++++++++++++++++++---
 gio/tests/gsubprocess.c |    3 ++
 2 files changed, 50 insertions(+), 4 deletions(-)
---
diff --git a/gio/gsubprocess.c b/gio/gsubprocess.c
index ca9b006..a2379dc 100644
--- a/gio/gsubprocess.c
+++ b/gio/gsubprocess.c
@@ -120,8 +120,8 @@
  *
  * During initable_init(), if the g_spawn() is successful then we
  * immediately register a child watch and take an extra ref on the
- * subprocess.  That reference doesn't drop until the child has quit,
- * which is why finalize can only happen in the non-running state.  In
+ * subprocess. Normally That reference doesn't drop until the child has
+ * quit, with the exception of when gio is unloaded or cleaned up. In
  * the event that the g_spawn() failed we will still be finalizing a
  * non-running GSubprocess (before returning from g_subprocess_new())
  * with NULL.
@@ -160,6 +160,8 @@ struct _GSubprocess
   GOutputStream *stdin_pipe;
   GInputStream  *stdout_pipe;
   GInputStream  *stderr_pipe;
+
+  gpointer cleanup_item;
 };
 
 G_DEFINE_TYPE_WITH_CODE (GSubprocess, g_subprocess, G_TYPE_OBJECT,
@@ -411,6 +413,9 @@ g_subprocess_exited (GPid     pid,
 
   g_spawn_close_pid (pid);
 
+  g_cleanup_list_remove (G_CLEANUP_LOCAL, self->cleanup_item);
+  self->cleanup_item = NULL;
+
   return FALSE;
 }
 
@@ -567,6 +572,16 @@ initable_init (GInitable     *initable,
       source = g_child_watch_source_new (self->pid);
       g_source_set_callback (source, (GSourceFunc) g_subprocess_exited, g_object_ref (self), g_object_unref);
       g_source_attach (source, worker_context);
+
+      /*
+       * Although normally we stay reffed until the child exits ... when gio is
+       * cleaned up, we need to go away (or we crash due to the glib main context
+       * running code in our address space.
+       */
+
+      self->cleanup_item = g_cleanup_list_push (G_CLEANUP_LOCAL, (GCleanupFunc)g_source_destroy, source,
+                                                G_CLEANUP_PHASE_EARLY, "GSubprocess.source");
+
       g_source_unref (source);
     }
 
@@ -591,15 +606,43 @@ static void
 g_subprocess_finalize (GObject *object)
 {
   GSubprocess *self = G_SUBPROCESS (object);
+  GSList *tasks;
 
-  g_assert (self->pending_waits == NULL);
-  g_assert (self->pid == 0);
+  /*
+   * Although in normal operation we'll never be finalized for
+   * a running child process ... this can be the case when gio
+   * is being cleaned up. So handle that gracefully.
+   */
+
+  g_mutex_lock (&self->pending_waits_lock);
+
+  self->status = 127;
+  tasks = self->pending_waits;
+  self->pending_waits = NULL;
+  if (self->pid)
+    {
+      g_spawn_close_pid (self->pid);
+      self->pid = 0;
+    }
+
+  g_mutex_unlock (&self->pending_waits_lock);
+
+  /* Signal anyone in g_subprocess_wait_async() to wake up now */
+  while (tasks)
+    {
+      g_task_return_new_error (tasks->data, G_IO_ERROR, G_IO_ERROR_CANCELLED,
+                               _("This process is shutting down"));
+      g_object_unref (tasks->data);
+      tasks = g_slist_delete_link (tasks, tasks);
+    }
 
   g_clear_object (&self->stdin_pipe);
   g_clear_object (&self->stdout_pipe);
   g_clear_object (&self->stderr_pipe);
   g_strfreev (self->argv);
 
+  g_cleanup_list_remove (G_CLEANUP_LOCAL, self->cleanup_item);
+
   G_OBJECT_CLASS (g_subprocess_parent_class)->finalize (object);
 }
 
diff --git a/gio/tests/gsubprocess.c b/gio/tests/gsubprocess.c
index f38bbdc..6e02da3 100644
--- a/gio/tests/gsubprocess.c
+++ b/gio/tests/gsubprocess.c
@@ -966,6 +966,9 @@ test_pass_fd (void)
 int
 main (int argc, char **argv)
 {
+  /* We're testing glib, not other (leaky) stuff */
+  g_setenv ("GIO_MODULE_DIR", "/non-existant", TRUE);
+
   g_test_init (&argc, &argv, NULL);
 
   g_test_add_func ("/gsubprocess/noop", test_noop);