[Spice-devel] [PATCH v5 05/13] usbredir: Spawn a different thread for device redirection flow

[Dmitry Fleytman]

From: Kirill Moizik <kmoizik at redhat.com>

On Windows when using usbdk, opening and closing USB device handle,
i.e. calling libusb_open()/libusb_unref_device() can block for a few
seconds (3-5 second more specifically on patch author's HW).

libusb_open() is called by spice_usbredir_channel_open_device().

libusb_unref_device() is called implicitely via
usbredirhost_set_device(..., NULL) from
spice_usbredir_channel_disconnect_device().

Both these calls happen on the main thread. If it blocks,
this causes the UI to freeze.

This commit makes sure libusb_open() is called from a different
thread to avoid blocking the mainloop freezes when usbdk is used.

Following commits also move usbredirhost_set_device(..., NULL) call
to the different threads.

Since this commit introduces additional execution contexts running in
parallell to the main thread threre are thread safety concerns to be secured.
Mainly there are 3 types of objects accessed by newly introduced threads:
  1. libusb contexts
  2. usbredir context
  3. redirection channels

Fortunately libusb accesses either thread safe of already performed by
a separate thread and protected by locks as needed.

As for channels and usbredir, in order to achieve thread safety this patch introduces
additional locks and references, namely:

1. To make sure channel objects are not disposed while redirection in progress,
   they are referenced on flow start and unreferenced on flow completion;
2. Channel objects data accesses from different threads protected with a
   new lock (flows_mutex);
3. Handling usbredir messages protected by the same new lock in order to
   ensure there are no messages processing flows in progres when device gets
   connected or disconneced.

Signed-off-by: Kirill Moizik <kmoizik at redhat.com>
Signed-off-by: Dmitry Fleytman <dfleytma at redhat.com>
---
 src/channel-usbredir.c | 41 ++++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
index c88322a..302a2aa 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
+_open_device_async_cb(GSimpleAsyncResult *simple,
+                      GObject *object,
+                      GCancellable *cancellable)
+{
+    GError *err = NULL;
+    SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(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,14 @@ void spice_usbredir_channel_connect_device_async(
                                           GAsyncReadyCallback   callback,
                                           gpointer              user_data)
 {
-    SpiceUsbredirChannelPrivate *priv = channel->priv;
+    SpiceUsbredirChannelPrivate *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);
 
+    priv = channel->priv;
+
     CHANNEL_DEBUG(channel, "connecting device %04x:%04x (%p) to channel %p",
                   spice_usb_device_get_vid(spice_device),
                   spice_usb_device_get_pid(spice_device),
@@ -372,13 +392,12 @@ 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,
+                                        _open_device_async_cb,
+                                        G_PRIORITY_DEFAULT,
+                                        cancellable);
+    g_object_unref(result);
+    return;
 #endif
 
 done:
-- 
2.4.3