Hi,
On Wed, Mar 23, 2016 at 01:04:03PM +0100, Fabiano Fidêncio wrote:
> gtask-helper provide methods that can easily be used for returning in
> idle, as a few issues have been found in the GTask code used in
> spice-gtk due to a immediately return when a return in idle was
> expected. As examples of these issues, you can take a look on commits
> 7774b8c and e81d97c.
* 7774b8c is "channel-usbredir: Fix crash due to a Task returning earlier
than expected". I think this kind of error is fine and we should just
fix it locally.
* e81d97c is "vmcstream,gtask: Do idle ourself instead of leaving it to
GTask's heuristic". This one is a true issue with our codebase +
g_task_return behavior as the callback function was not written
thinking that it would be called from a coroutine context.
Similar to e81d97c, the crash below also happens AFAIU because we are in
coroutine context.
<snip>
Program received signal SIGSEGV, Segmentation fault.
spice_file_transfer_task_completed (self=self@entry=0x7fffd0006f00, error=0x0) at channel-main.c:2963
2963 VDAgentFileXferStatusMessage msg = {
(gdb) bt
#0 spice_file_transfer_task_completed (self=self@entry=0x7fffd0006f00, error=0x0) at channel-main.c:2963
#1 in file_xfer_data_flushed_cb (source_object=0x7cc1d0, res=0x953390, user_data=user_data@entry=0x7fffd0006f00) at channel-main.c:1857
#2 in g_task_return_now (task=0x953390) at gtask.c:1108
#3 in g_task_return (task=0x953390, type=<optimized out>) at gtask.c:1166
#4 in flush_foreach_remove (key=<optimized out>, value=<optimized out>, user_data=<optimized out>) at channel-main.c:928
#5 in g_hash_table_foreach_remove_or_steal (hash_table=0x70cea0, func=func@entry=0x7ffff5616f10 <flush_foreach_remove>, user_data=user_data@entry=0x0, notify=notify@entry=1) at ghash.c:1492
#6 in g_hash_table_foreach_remove (hash_table=<optimized out>, func=func@entry=0x7ffff5616f10 <flush_foreach_remove>, user_data=user_data@entry=0x0) at ghash.c:1538
#7 in file_xfer_flushed (success=0, channel=0x7cc1d0) at channel-main.c:936
#8 spice_main_channel_reset_agent (channel=0x7cc1d0) at channel-main.c:466
#9 set_agent_connected (channel=0x7cc1d0, connected=connected@entry=0) at channel-main.c:1572
#10 in spice_main_channel_reset (channel=0x7cc1d0, migrating=0) at channel-main.c:485
#11 in spice_channel_coroutine (data=0x7cc1d0) at spice-channel.c:2564
#12 in coroutine_trampoline (cc=0x7cb860) at coroutine_ucontext.c:63
#13 in continuation_trampoline (i0=<optimized out>, i1=<optimized out>) at continuation.c:55
#14 in ?? () from /lib64/libc.so.6
#15 in ?? ()
#16 in ?? ()
Backtrace stopped: Cannot access memory at address
</snip>
(...)
With that said, I'd like to suggest a change in the gtask-helper.
Quoting the fine manual from GTask:
"The "return" methods (eg, g_task_return_pointer()) automatically cause
the task to be "completed" as well, and there is no need to worry about
the "complete" vs "complete in idle" distinction. (GTask automatically
figures out whether the task's callback can be invoked directly, or if
it needs to be sent to another GMainContext, or delayed until the next
iteration of the current GMainContext.)"
I would like that gtaskhelper do what the GTask can't at this moment
with the coroutine context which is "not make us worry about complete vs
complete in idle"
I would like to know what you think about changing
g_task_helper_return_int_in_idle to g_task_helper_return_int. The
non-idle version should then check if we are in the main-context using
coroutine_self_is_main(). If we are in the main-context, I would say we
can let g_task_return_int decide when to call the callback. If we are in
coroutine context, we can return in idle.
This approach would let us be closer to the gtask behavior/intention; we
would only be handling the coroutine context that gtask can't at the
moment.
PS: this should be able to fix the bug [0] on our end and we should open
a bug on glib/gio to discuss how we could extend g_task_return logic
upstream.
[0] https://bugs.freedesktop.org/show_bug.cgi?id=94662
Cheers,
toso
>
> Not all the functions introduced in gtask-helper.h are being used, but I
> still decided to add them for completeness reasons.
>
> Also, all the functions called in idle are the same that were being
> called in idle when using GSimpleAsyncResult. So, no issues should be
> found after this change and no behavior change should noticed.
>
> Signed-off-by: Fabiano Fidêncio <fidencio@xxxxxxxxxx>
> ---
> src/Makefile.am | 2 +
> src/channel-base.c | 4 +-
> src/channel-main.c | 12 ++--
> src/channel-usbredir.c | 12 ++--
> src/gtask-helper.c | 153 +++++++++++++++++++++++++++++++++++++++++++
> src/gtask-helper.h | 34 ++++++++++
> src/spice-channel.c | 4 +-
> src/spice-gstaudio.c | 6 +-
> src/usb-acl-helper.c | 12 ++--
> src/usb-device-manager.c | 18 ++---
> src/vmcstream.c | 35 ++--------
> src/win-usb-driver-install.c | 21 +++---
> 12 files changed, 246 insertions(+), 67 deletions(-)
> create mode 100644 src/gtask-helper.c
> create mode 100644 src/gtask-helper.h
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 66ba58b..783db37 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -235,6 +235,8 @@ libspice_client_glib_2_0_la_SOURCES = \
> coroutine.h \
> gio-coroutine.c \
> gio-coroutine.h \
> + gtask-helper.c \
> + gtask-helper.h \
> \
> channel-base.c \
> channel-webdav.c \
> diff --git a/src/channel-base.c b/src/channel-base.c
> index de04b89..67352e5 100644
> --- a/src/channel-base.c
> +++ b/src/channel-base.c
> @@ -17,6 +17,8 @@
> */
> #include "config.h"
>
> +#include "gtask-helper.h"
> +
> #include "spice-client.h"
> #include "spice-common.h"
>
> @@ -243,7 +245,7 @@ vmc_write_free_cb(uint8_t *data, void *user_data)
> GTask *task = user_data;
> gsize count = GPOINTER_TO_SIZE(g_task_get_task_data(task));
>
> - g_task_return_int(task, count);
> + g_task_helper_return_int_in_idle(task, count);
>
> g_object_unref(task);
> }
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 1c19de1..3873c45 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -21,6 +21,8 @@
> #include <spice/vd_agent.h>
> #include <glib/gstdio.h>
>
> +#include "gtask-helper.h"
> +
> #include "spice-client.h"
> #include "spice-common.h"
> #include "spice-marshal.h"
> @@ -923,7 +925,7 @@ static gboolean flush_foreach_remove(gpointer key G_GNUC_UNUSED,
> {
> gboolean success = GPOINTER_TO_UINT(user_data);
> GTask *result = value;
> - g_task_return_boolean(result, success);
> + g_task_helper_return_boolean_in_idle(result, success);
>
> return TRUE;
> }
> @@ -946,7 +948,7 @@ static void file_xfer_flush_async(SpiceMainChannel *channel, GCancellable *cance
>
> was_empty = g_queue_is_empty(c->agent_msg_queue);
> if (was_empty) {
> - g_task_return_boolean(task, TRUE);
> + g_task_helper_return_boolean_in_idle(task, TRUE);
> g_object_unref(task);
> return;
> }
> @@ -981,7 +983,7 @@ static void agent_send_msg_queue(SpiceMainChannel *channel)
> task = g_hash_table_lookup(c->flushing, out);
> if (task) {
> /* if there's a flush task waiting for this message, finish it */
> - g_task_return_boolean(task, TRUE);
> + g_task_helper_return_boolean_in_idle(task, TRUE);
> g_hash_table_remove(c->flushing, out);
> }
> }
> @@ -1790,9 +1792,9 @@ static void file_xfer_close_cb(GObject *object,
> self->priv->user_data);
>
> if (self->priv->error) {
> - g_task_return_error(task, self->priv->error);
> + g_task_helper_return_error_in_idle(task, self->priv->error);
> } else {
> - g_task_return_boolean(task, TRUE);
> + g_task_helper_return_boolean_in_idle(task, TRUE);
> if (spice_util_get_debug()) {
> gint64 now = g_get_monotonic_time();
> gchar *basename = g_file_get_basename(self->priv->file);
> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
> index ab90800..9549763 100644
> --- a/src/channel-usbredir.c
> +++ b/src/channel-usbredir.c
> @@ -32,6 +32,8 @@
> #include "usbutil.h"
> #endif
>
> +#include "gtask-helper.h"
> +
> #include "spice-client.h"
> #include "spice-common.h"
>
> @@ -301,9 +303,9 @@ static void spice_usbredir_channel_open_acl_cb(
> g_boxed_free(spice_usb_device_get_type(), priv->spice_device);
> priv->spice_device = NULL;
> priv->state = STATE_DISCONNECTED;
> - g_task_return_error(priv->task, err);
> + g_task_helper_return_error_in_idle(priv->task, err);
> } else {
> - g_task_return_boolean(priv->task, TRUE);
> + g_task_helper_return_boolean_in_idle(priv->task, TRUE);
> }
>
> g_clear_object(&priv->acl_helper);
> @@ -340,14 +342,14 @@ void spice_usbredir_channel_connect_device_async(
> task = g_task_new(channel, cancellable, callback, user_data);
>
> if (!priv->host) {
> - g_task_return_new_error(task,
> + g_task_helper_return_new_error_in_idle(task,
> SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> "Error libusb context not set");
> goto done;
> }
>
> if (priv->state != STATE_DISCONNECTED) {
> - g_task_return_new_error(task,
> + g_task_helper_return_new_error_in_idle(task,
> SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> "Error channel is busy");
> goto done;
> @@ -371,7 +373,7 @@ void spice_usbredir_channel_connect_device_async(
> return;
> #else
> if (!spice_usbredir_channel_open_device(channel, &err)) {
> - g_task_return_error(task, err);
> + g_task_helper_return_error_in_idle(task, err);
> libusb_unref_device(priv->device);
> priv->device = NULL;
> g_boxed_free(spice_usb_device_get_type(), priv->spice_device);
> diff --git a/src/gtask-helper.c b/src/gtask-helper.c
> new file mode 100644
> index 0000000..3c72396
> --- /dev/null
> +++ b/src/gtask-helper.c
> @@ -0,0 +1,153 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> +/*
> + Copyright (C) 2016 Red Hat, Inc.
> +
> + This library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.0 of the License, or (at your option) any later version.
> +
> + This library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with this library; if not, write to the Free Software
> + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> +*/
> +#include "config.h"
> +
> +#include "gtask-helper.h"
> +
> +typedef struct _GTaskHelperData
> +{
> + GTask *task;
> + GError *error;
> + gint integer;
> + gboolean boolean;
> + gpointer pointer;
> + GDestroyNotify destroy_notify_cb;
> +} GTaskHelperData;
> +
> +static GTaskHelperData *
> +g_task_helper_new(void)
> +{
> + return g_new0(GTaskHelperData, 1);
> +}
> +
> +static void
> +g_task_helper_data_free(GTaskHelperData *data)
> +{
> + g_clear_object(&data->task);
> + g_free(data);
> +}
> +
> +static gboolean
> +complete_in_idle_boolean_cb(gpointer user_data)
> +{
> + GTaskHelperData *data = user_data;
> +
> + g_task_return_boolean(data->task, data->boolean);
> + g_task_helper_data_free(data);
> +
> + return FALSE;
> +}
> +
> +static gboolean
> +complete_in_idle_int_cb(gpointer user_data)
> +{
> + GTaskHelperData *data = user_data;
> +
> + g_task_return_int(data->task, data->integer);
> + g_task_helper_data_free(data);
> +
> + return FALSE;
> +}
> +
> +static gboolean
> +complete_in_idle_error_cb(gpointer user_data)
> +{
> + GTaskHelperData *data = user_data;
> +
> + g_task_return_error(data->task, data->error);
> + g_task_helper_data_free(data);
> +
> + return FALSE;
> +}
> +
> +static gboolean
> +complete_in_idle_pointer_cb(gpointer user_data)
> +{
> + GTaskHelperData *data = user_data;
> +
> + g_task_return_pointer(data->task, data->pointer, data->destroy_notify_cb);
> + g_task_helper_data_free(data);
> +
> + return FALSE;
> +}
> +
> +void
> +g_task_helper_return_boolean_in_idle(GTask *task,
> + gboolean result)
> +{
> + GTaskHelperData *data = g_task_helper_new();
> + data->task = g_object_ref(task);
> + data->boolean = result;
> +
> + g_idle_add(complete_in_idle_boolean_cb, data);
> +}
> +
> +void
> +g_task_helper_return_int_in_idle(GTask *task,
> + gssize result)
> +{
> + GTaskHelperData *data = g_task_helper_new();
> + data->task = g_object_ref(task);
> + data->integer = result;
> +
> + g_idle_add(complete_in_idle_int_cb, data);
> +}
> +
> +
> +void
> +g_task_helper_return_error_in_idle(GTask *task,
> + GError *error)
> +{
> + GTaskHelperData *data = g_task_helper_new();
> + data->task = g_object_ref(task);
> + g_propagate_error(&data->error, error);
> +
> + g_idle_add(complete_in_idle_error_cb, data);
> +}
> +
> +
> +void __attribute__((format(gnu_printf, 4, 5)))
> +g_task_helper_return_new_error_in_idle(GTask *task,
> + GQuark domain,
> + gint code,
> + const char *format,
> + ...)
> +{
> + GTaskHelperData *data = g_task_helper_new();
> + va_list args;
> +
> + data->task = g_object_ref(task);
> + va_start(args, format);
> + data->error = g_error_new_valist(domain, code, format, args);
> + va_end(args);
> +
> + g_idle_add(complete_in_idle_error_cb, data);
> +}
> +
> +void g_task_helper_return_pointer_in_idle(GTask *task,
> + gpointer result,
> + GDestroyNotify result_destroy)
> +{
> + GTaskHelperData *data = g_task_helper_new();
> + data->task = g_object_ref(task);
> + data->pointer = result;
> + data->destroy_notify_cb = result_destroy;
> +
> + g_idle_add(complete_in_idle_pointer_cb, data);
> +}
> diff --git a/src/gtask-helper.h b/src/gtask-helper.h
> new file mode 100644
> index 0000000..81c041f
> --- /dev/null
> +++ b/src/gtask-helper.h
> @@ -0,0 +1,34 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> +/*
> + Copyright (C) 2016 Red Hat, Inc.
> +
> + This library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.0 of the License, or (at your option) any later version.
> +
> + This library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with this library; if not, write to the Free Software
> + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> +*/
> +#ifndef __G_TASK_HELPER_H__
> +#define __G_TASK_HELPER_H__
> +
> +#include <gio/gio.h>
> +
> +G_BEGIN_DECLS
> +
> +void g_task_helper_return_boolean_in_idle(GTask *task, gboolean result);
> +void g_task_helper_return_int_in_idle(GTask *task, gssize result);
> +void g_task_helper_return_error_in_idle(GTask *task, GError *error);
> +void g_task_helper_return_new_error_in_idle(GTask *task, GQuark domain, gint code, const char *format, ...);
> +void g_task_helper_return_pointer_in_idle(GTask *task, gpointer result, GDestroyNotify result_destroy);
> +
> +G_END_DECLS
> +
> +#endif /* __G_TASK_HELPER_H__ */
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index 8ae0e4d..fdbc45b 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -17,6 +17,8 @@
> */
> #include "config.h"
>
> +#include "gtask-helper.h"
> +
> #include "spice-client.h"
> #include "spice-common.h"
>
> @@ -2191,7 +2193,7 @@ static void spice_channel_flushed(SpiceChannel *channel, gboolean success)
> GSList *l;
>
> for (l = c->flushing; l != NULL; l = l->next) {
> - g_task_return_boolean(G_TASK(l->data), success);
> + g_task_helper_return_boolean_in_idle(G_TASK(l->data), success);
> }
>
> g_slist_free_full(c->flushing, g_object_unref);
> diff --git a/src/spice-gstaudio.c b/src/spice-gstaudio.c
> index a7c3c24..d810911 100644
> --- a/src/spice-gstaudio.c
> +++ b/src/spice-gstaudio.c
> @@ -22,6 +22,8 @@
> #include <gst/app/gstappsink.h>
> #include <gst/audio/streamvolume.h>
>
> +#include "gtask-helper.h"
> +
> #include "spice-gstaudio.h"
> #include "spice-common.h"
> #include "spice-session.h"
> @@ -572,7 +574,7 @@ static void spice_gstaudio_get_playback_volume_info_async(SpiceAudio *audio,
> {
> GTask *task = g_task_new(audio, cancellable, callback, user_data);
>
> - g_task_return_boolean(task, TRUE);
> + g_task_helper_return_boolean_in_idle(task, TRUE);
> }
>
> static gboolean spice_gstaudio_get_playback_volume_info_finish(SpiceAudio *audio,
> @@ -654,7 +656,7 @@ static void spice_gstaudio_get_record_volume_info_async(SpiceAudio *audio,
> {
> GTask *task = g_task_new(audio, cancellable, callback, user_data);
>
> - g_task_return_boolean(task, TRUE);
> + g_task_helper_return_boolean_in_idle(task, TRUE);
> }
>
> static gboolean spice_gstaudio_get_record_volume_info_finish(SpiceAudio *audio,
> diff --git a/src/usb-acl-helper.c b/src/usb-acl-helper.c
> index 487e1ee..5d03879 100644
> --- a/src/usb-acl-helper.c
> +++ b/src/usb-acl-helper.c
> @@ -25,6 +25,8 @@
> #include <stdio.h>
> #include <string.h>
>
> +#include "gtask-helper.h"
> +
> #include "usb-acl-helper.h"
>
> /* ------------------------------------------------------------------ */
> @@ -91,7 +93,7 @@ static void spice_usb_acl_helper_class_init(SpiceUsbAclHelperClass *klass)
>
> static void async_result_set_cancelled(GTask *task)
> {
> - g_task_return_new_error(task,
> + g_task_helper_return_new_error_in_idle(task,
> G_IO_ERROR, G_IO_ERROR_CANCELLED,
> "Setting USB device node ACL cancelled");
> }
> @@ -120,11 +122,11 @@ static gboolean cb_out_watch(GIOChannel *channel,
> string[strlen(string) - 1] = 0;
> if (!strcmp(string, "SUCCESS")) {
> success = TRUE;
> - g_task_return_boolean(priv->task, TRUE);
> + g_task_helper_return_boolean_in_idle(priv->task, TRUE);
> } else if (!strcmp(string, "CANCELED")) {
> async_result_set_cancelled(priv->task);
> } else {
> - g_task_return_new_error(priv->task,
> + g_task_helper_return_new_error_in_idle(priv->task,
> SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> "Error setting USB device node ACL: '%s'",
> string);
> @@ -132,10 +134,10 @@ static gboolean cb_out_watch(GIOChannel *channel,
> g_free(string);
> break;
> case G_IO_STATUS_ERROR:
> - g_task_return_error(priv->task, err);
> + g_task_helper_return_error_in_idle(priv->task, err);
> break;
> case G_IO_STATUS_EOF:
> - g_task_return_new_error(priv->task,
> + g_task_helper_return_new_error_in_idle(priv->task,
> SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> "Unexpected EOF reading from acl helper stdout");
> break;
> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> index 85231a1..eaaf6a4 100644
> --- a/src/usb-device-manager.c
> +++ b/src/usb-device-manager.c
> @@ -42,6 +42,8 @@
> #include "usbutil.h"
> #endif
>
> +#include "gtask-helper.h"
> +
> #include "spice-session-priv.h"
> #include "spice-client.h"
> #include "spice-marshal.h"
> @@ -1442,7 +1444,7 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
> guint i;
>
> if (spice_usb_device_manager_is_device_connected(self, device)) {
> - g_task_return_new_error(task,
> + g_task_helper_return_new_error_in_idle(task,
> SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> "Cannot connect an already connected usb device");
> goto done;
> @@ -1466,10 +1468,10 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
> g_ptr_array_remove(priv->devices, device);
> g_signal_emit(self, signals[DEVICE_REMOVED], 0, device);
> spice_usb_device_unref(device);
> - g_task_return_new_error(task,
> - SPICE_CLIENT_ERROR,
> - SPICE_CLIENT_ERROR_FAILED,
> - _("Device was not found"));
> + g_task_helper_return_new_error_in_idle(task,
> + SPICE_CLIENT_ERROR,
> + SPICE_CLIENT_ERROR_FAILED,
> + _("Device was not found"));
> goto done;
> }
> #endif
> @@ -1484,9 +1486,9 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
> }
> #endif
>
> - g_task_return_new_error(task,
> - SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> - _("No free USB channel"));
> + g_task_helper_return_new_error_in_idle(task,
> + SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> + _("No free USB channel"));
> #ifdef USE_USBREDIR
> done:
> #endif
> diff --git a/src/vmcstream.c b/src/vmcstream.c
> index 5dd2799..2df31e5 100644
> --- a/src/vmcstream.c
> +++ b/src/vmcstream.c
> @@ -19,6 +19,7 @@
>
> #include <string.h>
>
> +#include "gtask-helper.h"
> #include "vmcstream.h"
> #include "spice-channel-priv.h"
> #include "gio-coroutine.h"
> @@ -97,24 +98,6 @@ 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 gboolean
> -complete_in_idle_cb(gpointer user_data)
> -{
> - complete_in_idle_cb_data *data = user_data;
> -
> - g_task_return_int(data->task, data->pos);
> -
> - g_object_unref (data->task);
> - g_free (data);
> -
> - return FALSE;
> -}
> -
> /* coroutine */
> /**
> * Feed a SpiceVmc stream with new data from a coroutine
> @@ -134,8 +117,6 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream *self,
> self->coroutine = coroutine_self();
>
> while (size > 0) {
> - complete_in_idle_cb_data *cb_data;
> -
> SPICE_DEBUG("spicevmc co_data %p", self->task);
> if (!self->task)
> coroutine_yield(NULL);
> @@ -157,13 +138,7 @@ spice_vmc_input_stream_co_data(SpiceVmcInputStream *self,
> if (self->all && min > 0 && self->pos != self->count)
> continue;
>
> - /* Let's deal with the task complete in idle by ourselves, as GTask
> - * heuristic only makes sense in a non-coroutine case.
> - */
> - cb_data = g_new(complete_in_idle_cb_data , 1);
> - cb_data->task = g_object_ref(self->task);
> - cb_data->pos = self->pos;
> - g_idle_add(complete_in_idle_cb, cb_data);
> + g_task_helper_return_int_in_idle(self->task, self->pos);
>
> g_cancellable_disconnect(g_task_get_cancellable(self->task), self->cancel_id);
> g_clear_object(&self->task);
> @@ -179,9 +154,9 @@ read_cancelled(GCancellable *cancellable,
> SpiceVmcInputStream *self = SPICE_VMC_INPUT_STREAM(user_data);
>
> SPICE_DEBUG("read cancelled, %p", self->task);
> - g_task_return_new_error(self->task,
> - G_IO_ERROR, G_IO_ERROR_CANCELLED,
> - "read cancelled");
> + g_task_helper_return_new_error_in_idle(self->task,
> + G_IO_ERROR, G_IO_ERROR_CANCELLED,
> + "read cancelled");
>
> g_clear_object(&self->task);
>
> diff --git a/src/win-usb-driver-install.c b/src/win-usb-driver-install.c
> index 0d4627a..14de369 100644
> --- a/src/win-usb-driver-install.c
> +++ b/src/win-usb-driver-install.c
> @@ -31,6 +31,7 @@
> #include <gio/gio.h>
> #include <gio/gwin32inputstream.h>
> #include <gio/gwin32outputstream.h>
> +#include "gtask-helper.h"
> #include "spice-util.h"
> #include "win-usb-clerk.h"
> #include "win-usb-driver-install.h"
> @@ -143,13 +144,13 @@ void win_usb_driver_handle_reply_cb(GObject *gobject,
>
> if (err) {
> g_warning("failed to read reply from usbclerk (%s)", err->message);
> - g_task_return_error(priv->task, err);
> + g_task_helper_return_error_in_idle(priv->task, err);
> goto failed_reply;
> }
>
> if (bytes == 0) {
> g_warning("unexpected EOF from usbclerk");
> - g_task_return_new_error(priv->task,
> + g_task_helper_return_new_error_in_idle(priv->task,
> SPICE_WIN_USB_DRIVER_ERROR,
> SPICE_WIN_USB_DRIVER_ERROR_FAILED,
> "unexpected EOF from usbclerk");
> @@ -166,7 +167,7 @@ void win_usb_driver_handle_reply_cb(GObject *gobject,
> if (priv->reply.hdr.magic != USB_CLERK_MAGIC) {
> g_warning("usbclerk magic mismatch: mine=0x%04x server=0x%04x",
> USB_CLERK_MAGIC, priv->reply.hdr.magic);
> - g_task_return_new_error(priv->task,
> + g_task_helper_return_new_error_in_idle(priv->task,
> SPICE_WIN_USB_DRIVER_ERROR,
> SPICE_WIN_USB_DRIVER_ERROR_MESSAGE,
> "usbclerk magic mismatch");
> @@ -176,7 +177,7 @@ void win_usb_driver_handle_reply_cb(GObject *gobject,
> if (priv->reply.hdr.version != USB_CLERK_VERSION) {
> g_warning("usbclerk version mismatch: mine=0x%04x server=0x%04x",
> USB_CLERK_VERSION, priv->reply.hdr.version);
> - g_task_return_new_error(priv->task,
> + g_task_helper_return_new_error_in_idle(priv->task,
> SPICE_WIN_USB_DRIVER_ERROR,
> SPICE_WIN_USB_DRIVER_ERROR_MESSAGE,
> "usbclerk version mismatch");
> @@ -185,7 +186,7 @@ void win_usb_driver_handle_reply_cb(GObject *gobject,
> if (priv->reply.hdr.type != USB_CLERK_REPLY) {
> g_warning("usbclerk message with unexpected type %d",
> priv->reply.hdr.type);
> - g_task_return_new_error(priv->task,
> + g_task_helper_return_new_error_in_idle(priv->task,
> SPICE_WIN_USB_DRIVER_ERROR,
> SPICE_WIN_USB_DRIVER_ERROR_MESSAGE,
> "usbclerk message with unexpected type");
> @@ -195,7 +196,7 @@ void win_usb_driver_handle_reply_cb(GObject *gobject,
> if (priv->reply.hdr.size != bytes) {
> g_warning("usbclerk message size mismatch: read %"G_GSSIZE_FORMAT" bytes hdr.size=%d",
> bytes, priv->reply.hdr.size);
> - g_task_return_new_error(priv->task,
> + g_task_helper_return_new_error_in_idle(priv->task,
> SPICE_WIN_USB_DRIVER_ERROR,
> SPICE_WIN_USB_DRIVER_ERROR_MESSAGE,
> "usbclerk message with unexpected size");
> @@ -203,14 +204,14 @@ void win_usb_driver_handle_reply_cb(GObject *gobject,
> }
>
> if (priv->reply.status == 0) {
> - g_task_return_new_error(priv->task,
> + g_task_helper_return_new_error_in_idle(priv->task,
> SPICE_WIN_USB_DRIVER_ERROR,
> SPICE_WIN_USB_DRIVER_ERROR_MESSAGE,
> "usbclerk error reply");
> goto failed_reply;
> }
>
> - g_task_return_boolean (priv->task, TRUE);
> + g_task_helper_return_boolean_in_idle(priv->task, TRUE);
>
> failed_reply:
> g_clear_object(&priv->task);
> @@ -312,7 +313,7 @@ void spice_win_usb_driver_op(SpiceWinUsbDriver *self,
>
> if (priv->task) { /* allow one install/uninstall request at a time */
> g_warning("Another request exists -- try later");
> - g_task_return_new_error(task,
> + g_task_helper_return_new_error_in_idle(task,
> SPICE_WIN_USB_DRIVER_ERROR, SPICE_WIN_USB_DRIVER_ERROR_FAILED,
> "Another request exists -- try later");
> goto failed_request;
> @@ -325,7 +326,7 @@ void spice_win_usb_driver_op(SpiceWinUsbDriver *self,
> if (!spice_win_usb_driver_send_request(self, op_type,
> vid, pid, &err)) {
> g_warning("failed to send a request to usbclerk %s", err->message);
> - g_task_return_error(task, err);
> + g_task_helper_return_error_in_idle(task, err);
> goto failed_request;
> }
>
> --
> 2.5.5
>
> _______________________________________________
> 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