[Spice-devel] [PATCH v3 05/13] UsbDeviceManager and redirect channel: spawn a different thread for connect device flow
Christophe Fergeau
cfergeau at redhat.com
Tue Aug 11 04:22:03 PDT 2015
The commit log should explain _why_ this is needed ("on Windows when
using usbdk, calling libusb_xxxx can block for N seconds. libusb_xxxx is
called by usbredir_yyyy, which is called from the main thread. If it
blocks, this will cause the UI to freeze. This commit makes sure
usbredir_yyyy is called from a different thread to avoid blocking the
mainloop when usbdk is used.")
It should also explain why it's safe to call this from a different
thread.
Christophe
On Mon, Aug 03, 2015 at 04:10:45PM +0300, Kirill Moizik wrote:
> From: Kirill Moizik <kmoizik at redhat.com>
>
> ---
> src/channel-usbredir.c | 46 +++++++++++++++++++++++---------
> src/usb-device-manager.c | 69 +++++++++++++++++++++++++++++++++++++++---------
> 2 files changed, 90 insertions(+), 25 deletions(-)
>
> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
> index 69cb565..7394b81 100644
> --- a/src/channel-usbredir.c
> +++ b/src/channel-usbredir.c
> @@ -315,6 +315,27 @@ static void spice_usbredir_channel_open_acl_cb(
> }
> #endif
>
> +#ifndef USE_POLKIT
> +static void
> +spice_usbredir_channel_open_device_async(GSimpleAsyncResult *simple,
> + GObject *object,
> + GCancellable *cancellable)
> +{
> + GError *err = NULL;
> + SpiceUsbredirChannel *channel= (SpiceUsbredirChannel *)object;
> + SpiceUsbredirChannelPrivate *priv = channel->priv;
> + g_mutex_lock(priv->flows_mutex);
> + if (!spice_usbredir_channel_open_device(channel, &err)) {
> + g_simple_async_result_take_error(simple, err);
> + libusb_unref_device(priv->device);
> + priv->device = NULL;
> + g_boxed_free(spice_usb_device_get_type(), priv->spice_device);
> + priv->spice_device = NULL;
> + }
> + g_mutex_unlock(priv->flows_mutex);
> +}
> +#endif
> +
> G_GNUC_INTERNAL
> void spice_usbredir_channel_connect_device_async(
> SpiceUsbredirChannel *channel,
> @@ -324,15 +345,16 @@ void spice_usbredir_channel_connect_device_async(
> GAsyncReadyCallback callback,
> gpointer user_data)
> {
> - SpiceUsbredirChannelPrivate *priv = channel->priv;
> - GSimpleAsyncResult *result;
> -#if ! USE_POLKIT
> - GError *err = NULL;
> -#endif
>
> g_return_if_fail(SPICE_IS_USBREDIR_CHANNEL(channel));
> g_return_if_fail(device != NULL);
>
> + SpiceUsbredirChannelPrivate *priv = channel->priv;
> + GSimpleAsyncResult *result;
> +#ifndef USE_POLKIT
> + g_object_ref(channel);
> +#endif
> +
> CHANNEL_DEBUG(channel, "connecting usb channel %p", channel);
>
> result = g_simple_async_result_new(G_OBJECT(channel), callback, user_data,
> @@ -369,18 +391,18 @@ void spice_usbredir_channel_connect_device_async(
> channel);
> return;
> #else
> - if (!spice_usbredir_channel_open_device(channel, &err)) {
> - g_simple_async_result_take_error(result, err);
> - libusb_unref_device(priv->device);
> - priv->device = NULL;
> - g_boxed_free(spice_usb_device_get_type(), priv->spice_device);
> - priv->spice_device = NULL;
> - }
> + g_simple_async_result_run_in_thread(result,
> + spice_usbredir_channel_open_device_async,
> + G_PRIORITY_DEFAULT,
> + cancellable);
> + g_object_unref(result);
> + return;
> #endif
>
> done:
> g_simple_async_result_complete_in_idle(result);
> g_object_unref(result);
> + g_object_unref(channel);
> }
>
> G_GNUC_INTERNAL
> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> index e58450d..65c6568 100644
> --- a/src/usb-device-manager.c
> +++ b/src/usb-device-manager.c
> @@ -207,6 +207,11 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
> GAsyncReadyCallback callback,
> gpointer user_data);
>
> +static
> +void spice_usb_device_manager_connect_device_async_cb(GObject *gobject,
> + GAsyncResult *channel_res,
> + gpointer user_data);
> +
> G_DEFINE_BOXED_TYPE(SpiceUsbDevice, spice_usb_device,
> (GBoxedCopyFunc)spice_usb_device_ref,
> (GBoxedFreeFunc)spice_usb_device_unref)
> @@ -1128,8 +1133,16 @@ static void spice_usb_device_manager_channel_connect_cb(
> }
> g_simple_async_result_complete(result);
> g_object_unref(result);
> + g_object_unref(channel);
> }
>
> +typedef struct _connect_cb_data
> +{
> + SpiceUsbDeviceManager *self;
> + GAsyncReadyCallback callback;
> + gpointer user_data;
> +} connect_cb_data;
> +
> #ifdef G_OS_WIN32
>
> typedef struct _UsbInstallCbInfo {
> @@ -1165,7 +1178,7 @@ static void spice_usb_device_manager_drv_install_cb(GObject *gobject,
> SpiceUsbDevice *device;
> UsbInstallCbInfo *cbinfo;
> GCancellable *cancellable;
> - GAsyncReadyCallback callback;
> + connect_cb_data *data;
>
> g_return_if_fail(user_data != NULL);
>
> @@ -1174,8 +1187,7 @@ static void spice_usb_device_manager_drv_install_cb(GObject *gobject,
> device = cbinfo->device;
> installer = cbinfo->installer;
> cancellable = cbinfo->cancellable;
> - callback = cbinfo->callback;
> - user_data = cbinfo->user_data;
> + data = cbinfo->user_data;
>
> g_free(cbinfo);
>
> @@ -1197,8 +1209,8 @@ static void spice_usb_device_manager_drv_install_cb(GObject *gobject,
> _spice_usb_device_manager_connect_device_async(self,
> device,
> cancellable,
> - callback,
> - user_data);
> + spice_usb_device_manager_connect_device_async_cb,
> + data);
>
> spice_usb_device_unref(device);
> }
> @@ -1485,6 +1497,7 @@ gboolean spice_usb_device_manager_is_device_connected(SpiceUsbDeviceManager *sel
> return !!spice_usb_device_manager_get_channel_for_dev(self, device);
> }
>
> +#ifdef USE_USBREDIR
> /**
> * spice_usb_device_manager_connect_device_async:
> * @manager: the #SpiceUsbDeviceManager manager
> @@ -1510,7 +1523,6 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
> result = g_simple_async_result_new(G_OBJECT(self), callback, user_data,
> spice_usb_device_manager_connect_device_async);
>
> -#ifdef USE_USBREDIR
> SpiceUsbDeviceManagerPrivate *priv = self->priv;
> libusb_device *libdev;
> guint i;
> @@ -1556,18 +1568,16 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
> libusb_unref_device(libdev);
> return;
> }
> -#endif
>
> g_simple_async_result_set_error(result,
> SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> _("No free USB channel"));
> -#ifdef USE_USBREDIR
> done:
> -#endif
> g_simple_async_result_complete_in_idle(result);
> g_object_unref(result);
> }
>
> +#endif
>
> void spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
> SpiceUsbDevice *device,
> @@ -1575,8 +1585,20 @@ void spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
> GAsyncReadyCallback callback,
> gpointer user_data)
> {
> + g_return_if_fail(SPICE_IS_USB_DEVICE_MANAGER(self));
> +#ifdef USE_USBREDIR
> + g_return_if_fail(self->priv->redirecting == FALSE);
> + g_object_set(self,"redirecting", TRUE, NULL);
> +#ifndef USE_LIBUSB_HOTPLUG
> + spice_g_udev_set_redirecting(TRUE);
> +#endif
> + connect_cb_data *data = g_new(connect_cb_data,1);
> + data->self = self;
> + data->callback = callback;
> + data->user_data = user_data;
> +
> if (self->priv->use_usbclerk) {
> -#if defined(USE_USBREDIR) && defined(G_OS_WIN32)
> +#ifdef G_OS_WIN32
> SpiceWinUsbDriver *installer;
> UsbInstallCbInfo *cbinfo;
>
> @@ -1591,7 +1613,7 @@ void spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
> cbinfo->installer = installer;
> cbinfo->cancellable = cancellable;
> cbinfo->callback = callback;
> - cbinfo->user_data = user_data;
> + cbinfo->user_data = data;
>
> spice_win_usb_driver_install_async(installer, device, cancellable,
> spice_usb_device_manager_drv_install_cb,
> @@ -1603,9 +1625,10 @@ void spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
> _spice_usb_device_manager_connect_device_async(self,
> device,
> cancellable,
> - callback,
> - user_data);
> + spice_usb_device_manager_connect_device_async_cb,
> + data);
> }
> +#endif
> }
>
> gboolean spice_usb_device_manager_connect_device_finish(
> @@ -1623,6 +1646,26 @@ gboolean spice_usb_device_manager_connect_device_finish(
> return TRUE;
> }
>
> +#ifdef USE_USBREDIR
> +static
> +void spice_usb_device_manager_connect_device_async_cb(GObject *gobject,
> + GAsyncResult *channel_res,
> + gpointer user_data)
> +{
> + connect_cb_data *data = user_data;
> + GSimpleAsyncResult *result = g_simple_async_result_new(G_OBJECT(data->self), data->callback,
> + data->user_data,
> + spice_usb_device_manager_disconnect_device_async);
> + g_object_set(data->self,"redirecting", FALSE, NULL);
> +#ifndef USE_LIBUSB_HOTPLUG
> + spice_g_udev_set_redirecting(FALSE);
> +#endif
> + g_simple_async_result_complete(result);
> + g_object_unref(result);
> + g_free(data);
> +}
> +#endif
> +
> /**
> * spice_usb_device_manager_disconnect_device:
> * @manager: the #SpiceUsbDeviceManager manager
> --
> 2.4.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150811/8f683188/attachment-0001.sig>
More information about the Spice-devel
mailing list