On Mon, 2016-05-02 at 15:16 +0200, Victor Toso wrote: > spicevmc was designed so its _finish functions should always be called; > With the gtask integration both _finish functions are trying to > disconnect the GCancellabe even in the 'cancel' context which leads to > a deadlock as explained in the documentation. > > Resolves: https://bugs.freedesktop.org/show_bug.cgi?id=95032 > --- > src/vmcstream.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/src/vmcstream.c b/src/vmcstream.c > index 5dd2799..a8bedf5 100644 > --- a/src/vmcstream.c > +++ b/src/vmcstream.c > @@ -183,13 +183,10 @@ read_cancelled(GCancellable *cancellable, > G_IO_ERROR, G_IO_ERROR_CANCELLED, > "read cancelled"); > > + /* With GTask, we don't need to deal with GCancellable when task is > + * cancelled within cancellable callback as it could lead to deadlocks > + * e.g: https://bugzilla.gnome.org/show_bug.cgi?id=705395 */ the phrase "deal with" is a bit too vague. I'd say something like "we don't need to disconnect..." instead perhaps? Otherwise I think this patch looks fine. Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx> > g_clear_object(&self->task); > - > - /* See FIXME */ > - /* if (self->cancellable) { */ > - /* g_cancellable_disconnect(self->cancellable, self->cancel_id); */ > - /* g_clear_object(&self->cancellable); */ > - /* } */ > } > > G_GNUC_INTERNAL void > @@ -230,13 +227,14 @@ spice_vmc_input_stream_read_all_finish(GInputStream > *stream, > { > GTask *task = G_TASK(result); > SpiceVmcInputStream *self = SPICE_VMC_INPUT_STREAM(stream); > + GCancellable *cancel; > > g_return_val_if_fail(g_task_is_valid(task, self), -1); > - > - /* FIXME: calling _finish() is required. Disconnecting in > - read_cancelled() causes a deadlock. #705395 */ > - g_cancellable_disconnect(g_task_get_cancellable(task), self->cancel_id); > - > + cancel = g_task_get_cancellable(task); > + if (!g_cancellable_is_cancelled(cancel)) { > + g_cancellable_disconnect(cancel, self->cancel_id); > + self->cancel_id = 0; > + } > return g_task_propagate_int(task, error); > } > > @@ -276,13 +274,15 @@ spice_vmc_input_stream_read_finish(GInputStream *stream, > { > GTask *task = G_TASK(result); > SpiceVmcInputStream *self = SPICE_VMC_INPUT_STREAM(stream); > + GCancellable *cancel; > > g_return_val_if_fail(g_task_is_valid(task, self), -1); > > - /* FIXME: calling _finish() is required. Disconnecting in > - read_cancelled() causes a deadlock. #705395 */ > - g_cancellable_disconnect(g_task_get_cancellable(task), self->cancel_id); > - > + cancel = g_task_get_cancellable(task); > + if (!g_cancellable_is_cancelled(cancel)) { > + g_cancellable_disconnect(cancel, self->cancel_id); > + self->cancel_id = 0; > + } > return g_task_propagate_int(task, error); > } > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel