From 8f86d312d8437d20d3d1f703ed2b270cee1803ea Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 23 Aug 2017 11:21:38 +0100 Subject: [PATCH] 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 https://bugzilla.gnome.org/show_bug.cgi?id=786456 --- gio/gsubprocess.c | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/gio/gsubprocess.c b/gio/gsubprocess.c index 350eb1e93..8525f3bfc 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 */ + } } /**