<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class="">
<br class=""><div><blockquote type="cite" class=""><div class="">On 23 Sep 2015, at 15:08 PM, Christophe Fergeau <<a href="mailto:cfergeau@redhat.com" class="">cfergeau@redhat.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On Sun, Aug 16, 2015 at 03:35:44PM +0300, Dmitry Fleytman wrote:</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">From: Kirill Moizik <<a href="mailto:kmoizik@redhat.com" class="">kmoizik@redhat.com</a>><br class=""><br class="">On Windows when using usbdk, opening and closing USB device handle,<br class="">i.e. calling libusb_open()/libusb_unref_device() can block for a few<br class="">seconds (3-5 second more specifically on patch author's HW).<br class=""><br class="">libusb_open() is called by spice_usbredir_channel_open_device().<br class=""><br class="">libusb_unref_device() is called implicitely via<br class="">usbredirhost_set_device(..., NULL) from<br class="">spice_usbredir_channel_disconnect_device().<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Is this libusb_unref_device() or libusb_close() which can block?</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">spice_usbredir_channel_disconnect_device() does:</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">/* This also closes the libusb handle we passed from open_device */</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">usbredirhost_set_device(priv->host, NULL);</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">libusb_unref_device(priv->device);</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">so the libusb_close() call is done by usbredirhost_set_device() while</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">the libusb_unref_device() call is done explicitly in</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">spice_usbredir_channel_disconnect_device()</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">If what is blocking is libusb_unref_device(), I would not bother making</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">disconnect() async by running in a thread, but I'd just spawn a thread</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">to do the libusb_unref_device() call. I don't think it's important that</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">we wait until the call completes, so _disconnect() would not need to</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">change apart from this delayed unref.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">If the blocking call is libusb_close(), I don't know if the same</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">approach can be used with usbredirhost_set_device? Has this been tried?</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div><br class=""></div><div>The blocking call is libusb_close().</div><div>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...</div><br class=""><blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Christophe</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class="">Both these calls happen on the main thread. If it blocks,<br class="">this causes the UI to freeze.<br class=""><br class="">This commit makes sure libusb_open() is called from a different<br class="">thread to avoid blocking the mainloop freezes when usbdk is used.<br class=""><br class="">Following commits also move usbredirhost_set_device(..., NULL) call<br class="">to the different threads.<br class=""><br class="">Since this commit introduces additional execution contexts running in<br class="">parallell to the main thread threre are thread safety concerns to be secured.<br class="">Mainly there are 3 types of objects accessed by newly introduced threads:<br class=""> 1. libusb contexts<br class=""> 2. usbredir context<br class=""> 3. redirection channels<br class=""><br class="">Fortunately libusb accesses either thread safe of already performed by<br class="">a separate thread and protected by locks as needed.<br class=""><br class="">As for channels and usbredir, in order to achieve thread safety this patch introduces<br class="">additional locks and references, namely:<br class=""><br class="">1. To make sure channel objects are not disposed while redirection in progress,<br class=""> they are referenced on flow start and unreferenced on flow completion;<br class="">2. Channel objects data accesses from different threads protected with a<br class=""> new lock (flows_mutex);<br class="">3. Handling usbredir messages protected by the same new lock in order to<br class=""> ensure there are no messages processing flows in progres when device gets<br class=""> connected or disconneced.<br class=""><br class="">Signed-off-by: Kirill Moizik <<a href="mailto:kmoizik@redhat.com" class="">kmoizik@redhat.com</a>><br class="">Signed-off-by: Dmitry Fleytman <<a href="mailto:dfleytma@redhat.com" class="">dfleytma@redhat.com</a>><br class="">---<br class="">src/channel-usbredir.c | 41 ++++++++++++++++++++++++++++++-----------<br class="">1 file changed, 30 insertions(+), 11 deletions(-)<br class=""><br class="">diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c<br class="">index bbfd2b0..44da3ce 100644<br class="">--- a/src/channel-usbredir.c<br class="">+++ b/src/channel-usbredir.c<br class="">@@ -315,6 +315,27 @@ static void spice_usbredir_channel_open_acl_cb(<br class="">}<br class="">#endif<br class=""><br class="">+#ifndef USE_POLKIT<br class="">+static void<br class="">+_open_device_async_cb(GSimpleAsyncResult *simple,<br class="">+ GObject *object,<br class="">+ GCancellable *cancellable)<br class="">+{<br class="">+ GError *err = NULL;<br class="">+ SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(object);<br class="">+ SpiceUsbredirChannelPrivate *priv = channel->priv;<br class="">+ g_mutex_lock(priv->flows_mutex);<br class="">+ if (!spice_usbredir_channel_open_device(channel, &err)) {<br class="">+ g_simple_async_result_take_error(simple, err);<br class="">+ libusb_unref_device(priv->device);<br class="">+ priv->device = NULL;<br class="">+ g_boxed_free(spice_usb_device_get_type(), priv->spice_device);<br class="">+ priv->spice_device = NULL;<br class="">+ }<br class="">+ g_mutex_unlock(priv->flows_mutex);<br class="">+}<br class="">+#endif<br class="">+<br class="">G_GNUC_INTERNAL<br class="">void spice_usbredir_channel_connect_device_async(<br class=""> SpiceUsbredirChannel *channel,<br class="">@@ -324,15 +345,14 @@ void spice_usbredir_channel_connect_device_async(<br class=""> GAsyncReadyCallback callback,<br class=""> gpointer user_data)<br class="">{<br class="">- SpiceUsbredirChannelPrivate *priv = channel->priv;<br class="">+ SpiceUsbredirChannelPrivate *priv;<br class=""> GSimpleAsyncResult *result;<br class="">-#if ! USE_POLKIT<br class="">- GError *err = NULL;<br class="">-#endif<br class=""><br class=""> g_return_if_fail(SPICE_IS_USBREDIR_CHANNEL(channel));<br class=""> g_return_if_fail(device != NULL);<br class=""><br class="">+ priv = channel->priv;<br class="">+<br class=""> CHANNEL_DEBUG(channel, "connecting usb channel %p", channel);<br class=""><br class=""> result = g_simple_async_result_new(G_OBJECT(channel), callback, user_data,<br class="">@@ -369,13 +389,12 @@ void spice_usbredir_channel_connect_device_async(<br class=""> channel);<br class=""> return;<br class="">#else<br class="">- if (!spice_usbredir_channel_open_device(channel, &err)) {<br class="">- g_simple_async_result_take_error(result, err);<br class="">- libusb_unref_device(priv->device);<br class="">- priv->device = NULL;<br class="">- g_boxed_free(spice_usb_device_get_type(), priv->spice_device);<br class="">- priv->spice_device = NULL;<br class="">- }<br class="">+ g_simple_async_result_run_in_thread(result,<br class="">+ _open_device_async_cb,<br class="">+ G_PRIORITY_DEFAULT,<br class="">+ cancellable);<br class="">+ g_object_unref(result);<br class="">+ return;<br class="">#endif<br class=""><br class="">done:<br class="">--<span class="Apple-converted-space"> </span><br class="">2.4.3<br class=""><br class="">_______________________________________________<br class="">Spice-devel mailing list<br class=""><a href="mailto:Spice-devel@lists.freedesktop.org" class="">Spice-devel@lists.freedesktop.org</a><br class=""><a href="http://lists.freedesktop.org/mailman/listinfo/spice-devel" class="">http://lists.freedesktop.org/mailman/listinfo/spice-devel</a></blockquote></div></blockquote></div><br class=""></body></html>