[glib] gio: Fix double-callback on cancellation with GSubprocess



commit 8f86d312d8437d20d3d1f703ed2b270cee1803ea
Author: Philip Withnall <withnall endlessm com>
Date:   Wed Aug 23 11:21:38 2017 +0100

    gio: Fix double-callback on cancellation with GSubprocess
    
    See bug #786456 for a detailed analysis of the situation which can cause
    this (in summary, if a g_subprocess_wait_async() call is cancelled on a
    GSubprocess which is already known to be dead).
    
    The problem was that the GCancellable callback handler was
    unconditionally returning a result for the GTask for
    g_subprocess_wait_async(), even if that GTask had already returned a
    result and the callback was being invoked after the GTask had been
    removed from the pending_waits list.
    
    Fix that by checking whether the GTask is still in the pending_waits
    list before returning a result for it.
    
    Thanks to Will Thompson for some very useful unit tests which reproduce
    this (which will be pushed in the following commit).
    
    Signed-off-by: Philip Withnall <withnall endlessm com>
    
    https://bugzilla.gnome.org/show_bug.cgi?id=786456

 gio/gsubprocess.c |   36 +++++++++++++++++++++++++++++++++---
 1 files changed, 33 insertions(+), 3 deletions(-)
---
diff --git a/gio/gsubprocess.c b/gio/gsubprocess.c
index 350eb1e..8525f3b 100644
--- a/gio/gsubprocess.c
+++ b/gio/gsubprocess.c
@@ -830,21 +830,51 @@ g_subprocess_get_stderr_pipe (GSubprocess *subprocess)
   return subprocess->stderr_pipe;
 }
 
+/* Remove the first list element containing @data, and return %TRUE. If no
+ * such element is found, return %FALSE. */
+static gboolean
+slist_remove_if_present (GSList        **list,
+                         gconstpointer   data)
+{
+  GSList *l, *prev;
+
+  for (l = *list, prev = NULL; l != NULL; prev = l, l = prev->next)
+    {
+      if (l->data == data)
+        {
+          if (prev != NULL)
+            prev->next = l->next;
+          else
+            *list = l->next;
+
+          g_slist_free_1 (l);
+
+          return TRUE;
+        }
+    }
+
+  return FALSE;
+}
+
 static void
 g_subprocess_wait_cancelled (GCancellable *cancellable,
                              gpointer      user_data)
 {
   GTask *task = user_data;
   GSubprocess *self;
+  gboolean task_was_pending;
 
   self = g_task_get_source_object (task);
 
   g_mutex_lock (&self->pending_waits_lock);
-  self->pending_waits = g_slist_remove (self->pending_waits, task);
+  task_was_pending = slist_remove_if_present (&self->pending_waits, task);
   g_mutex_unlock (&self->pending_waits_lock);
 
-  g_task_return_boolean (task, FALSE);
-  g_object_unref (task);
+  if (task_was_pending)
+    {
+      g_task_return_boolean (task, FALSE);
+      g_object_unref (task);  /* ref from pending_waits */
+    }
 }
 
 /**


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