Hi, On Mon, May 02, 2016 at 11:56:54AM -0500, Jonathon Jongsma wrote: > 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? Sure! > > Otherwise I think this patch looks fine. > > Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx> Thanks, pushed! > > > > 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 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel