[Spice-devel] [PATCH spice-gtk 1/2] vmcstream: fix task cancellation

[Victor Toso]

Hi,

On Tue, Aug 20, 2019 at 04:05:46PM +0200, Jakub Janků wrote:
> If the task is cancelled, g_task_propagate_pointer()
> in spice_vmc_output_stream_write_finish() returns NULL
> which produces following errors:
> 
>     (spicy:13597): GSpice-CRITICAL **: 15:39:46.168: spice_vmc_write_finish: assertion 'result != NULL' failed
> 
>     (spicy:13597): GLib-GObject-CRITICAL **: 15:39:46.168: g_object_unref: assertion 'G_IS_OBJECT (object)' failed

What I found weird was actually that we do _ref() on write_cb()
but this got cleanup anyway, seems like a bug somewhere.

I don't see nothing wrong with your proposal, actually thinks it
is better to do the write on write_cb() and let the _finish()
just provide the overall feedback of the _async() call.

> To fix this, call spice_vmc_write_finish() immediately in the
> corresponding write_cb().
> 
> Signed-off-by: Jakub Janků <jjanku at redhat.com>

Acked-by: Victor Toso <victortoso at redhat.com>

> ---
>  src/vmcstream.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/src/vmcstream.c b/src/vmcstream.c
> index b6f6d1a..b7da103 100644
> --- a/src/vmcstream.c
> +++ b/src/vmcstream.c
> @@ -397,18 +397,11 @@ spice_vmc_output_stream_write_fn(GOutputStream   *stream,
>  
>  static gssize
>  spice_vmc_output_stream_write_finish(GOutputStream *stream,
> -                                     GAsyncResult *simple,
> +                                     GAsyncResult *result,
>                                       GError **error)
>  {
> -    SpiceVmcOutputStream *self = SPICE_VMC_OUTPUT_STREAM(stream);
> -    GAsyncResult *res = g_task_propagate_pointer(G_TASK(simple), error);
> -    gssize bytes_written;
> -
> -    SPICE_DEBUG("spicevmc write finish");
> -    bytes_written = spice_vmc_write_finish(self->channel, res, error);
> -    g_object_unref(res);
> -
> -    return bytes_written;
> +    g_return_val_if_fail(g_task_is_valid(result, stream), -1);
> +    return g_task_propagate_int(G_TASK(result), error);
>  }
>  
>  static void
> @@ -417,9 +410,17 @@ write_cb(GObject *source_object,
>           gpointer user_data)
>  {
>      GTask *task = user_data;
> +    GError *error = NULL;
> +    gssize bytes_written;
>  
> -    g_task_return_pointer(task, g_object_ref(res), g_object_unref);
> +    SPICE_DEBUG("spicevmc write finish");
> +    bytes_written = spice_vmc_write_finish(SPICE_CHANNEL(source_object), res, &error);
>  
> +    if (error) {
> +        g_task_return_error(task, error);
> +    } else {
> +        g_task_return_int(task, bytes_written);
> +    }
>      g_object_unref(task);
>  }
>  
> -- 
> 2.21.0
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190822/6cc61542/attachment.sig>