[Spice-devel] [spice-gtk] vmcstream, gtask: Do idle ourself as GTask's return gets messed up

[Marc-André Lureau]

Hi

----- Original Message -----
> Seems that our coroutines can mess up with GTask's return, ending up
> in an early return from g_task_return_now() instead of doing a return
> in idle.
> As I still don't know if the bug is on spice-gtk or on GTask itself,
> I've opened a bug[0] in spice-gtk's bugzilla and also made a reference
> to the very same bug in the code.
> 
> [0]: https://bugs.freedesktop.org/show_bug.cgi?id=94662
> 
> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>

Is it easy to reproduce the issue? Just use webdav channel? Is there a crash & backtrace? 

> ---
>  src/vmcstream.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/src/vmcstream.c b/src/vmcstream.c
> index 5ebf15a..4db94c7 100644
> --- a/src/vmcstream.c
> +++ b/src/vmcstream.c
> @@ -97,6 +97,30 @@ spice_vmc_input_stream_new(void)
>      return self;
>  }
>  
> +typedef struct _complete_in_idle_cb_data {
> +    GTask *task;
> +    gssize pos;
> +} complete_in_idle_cb_data;
> +
> +static void
> +complete_in_idle_cb_data_free (gpointer user_data)
> +{
> +    complete_in_idle_cb_data *data = user_data;
> +
> +    g_object_unref (data->task);
> +    g_free (data);
> +}
> +
> +static gboolean
> +complete_in_idle_cb(gpointer user_data)
> +{
> +    complete_in_idle_cb_data *data = user_data;
> +
> +    g_task_return_int(data->task, data->pos);
> +
> +    return FALSE;
> +}
> +
>  /* coroutine */
>  /**
>   * Feed a SpiceVmc stream with new data from a coroutine
> @@ -116,6 +140,9 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream *self,
>      self->coroutine = coroutine_self();
>  
>      while (size > 0) {
> +        complete_in_idle_cb_data *cb_data;
> +        GSource *source;
> +
>          SPICE_DEBUG("spicevmc co_data %p", self->task);
>          if (!self->task)
>              coroutine_yield(NULL);
> @@ -137,10 +164,25 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream
> *self,
>          if (self->all && min > 0 && self->pos != self->count)
>              continue;
>  
> -        g_task_return_int(self->task, self->pos);
> +        /* Somehow we are messing up with GTask return. As it's not known
> +         * for now if it's a spice-gtk issue due to the coroutine usage or
> +         * a GTask issue itself, let's do the idle ourself.
> +         *
> +         * For a future reference, see:
> +         * https://bugs.freedesktop.org/show_bug.cgi?id=94662
> +         */
> +        cb_data = g_new(complete_in_idle_cb_data , 1);
> +        cb_data->task = g_object_ref(self->task);
> +        cb_data->pos = self->pos;
> +
> +        source = g_idle_source_new ();
> +        g_source_set_priority (source, G_PRIORITY_DEFAULT);
> +        g_source_set_callback (source, complete_in_idle_cb, cb_data,
> complete_in_idle_cb_data_free);
> +        g_source_attach (source, g_task_get_context(self->task));
> +        g_source_set_name (source, "[spice-gtk] complete_in_idle_cb");
> +        g_source_unref (source);

Why not use just g_idle_add()? 

>  
>          g_cancellable_disconnect(g_task_get_cancellable(self->task),
>          self->cancel_id);
> -
>          g_clear_object(&self->task);
>      }
>  
> --
> 2.5.0
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>