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
This commit is contained in:
Philip Withnall 2017-08-23 11:21:38 +01:00
parent a60359aee2
commit 8f86d312d8

View File

@ -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 */
}
}
/**