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

[Stefan Walter <stefw src gnome org>]

commit ec195e162865189f3e8b976d17e813aa5aae6d9f
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.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=711799

 gio/gsubprocess.c       |   52 +++++++++++++++++++++++++++++++++++++++++++---
 gio/tests/gsubprocess.c |    3 ++
 2 files changed, 51 insertions(+), 4 deletions(-)
---
diff --git a/gio/gsubprocess.c b/gio/gsubprocess.c
index ca9b006..aa80136 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_SCOPE, self->cleanup_item);
+  self->cleanup_item = NULL;
+
   return FALSE;
 }
 
@@ -567,6 +572,17 @@ 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_SCOPE, G_CLEANUP_PHASE_EARLY,
+                                                (GCleanupFunc)g_source_destroy, source,
+                                                "GSubprocess.source");
+
       g_source_unref (source);
     }
 
@@ -591,15 +607,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_SCOPE, 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);