On Sun, Aug 16, 2015 at 03:35:44PM +0300, Dmitry Fleytman wrote:From: Kirill Moizik <kmoizik@xxxxxxxxxx>
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().
Is this libusb_unref_device() or libusb_close() which can block?spice_usbredir_channel_disconnect_device() does:/* This also closes the libusb handle we passed from open_device */usbredirhost_set_device(priv->host, NULL);libusb_unref_device(priv->device);so the libusb_close() call is done by usbredirhost_set_device() whilethe libusb_unref_device() call is done explicitly inspice_usbredir_channel_disconnect_device()If what is blocking is libusb_unref_device(), I would not bother makingdisconnect() async by running in a thread, but I'd just spawn a threadto do the libusb_unref_device() call. I don't think it's important thatwe wait until the call completes, so _disconnect() would not need tochange apart from this delayed unref.If the blocking call is libusb_close(), I don't know if the sameapproach can be used with usbredirhost_set_device? Has this been tried?
The blocking call is libusb_close(). It is safer to wait for device close operation to complete, otherwise re-connection to the same device may be initiated while close is still in progress which may bring execution to a different corner cases... Christophe
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@xxxxxxxxxx>
Signed-off-by: Dmitry Fleytman <dfleytma@xxxxxxxxxx>
---
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 bbfd2b0..44da3ce 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 usb channel %p", channel);
result = g_simple_async_result_new(G_OBJECT(channel), callback, user_data,
@@ -369,13 +389,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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel
|