[Spice-devel] [PATCH v2 6/6] Make usb redirection stop and surprise removal flows asynchronous

[Kirill Moizik]

On Wed, Jul 8, 2015 at 12:07 PM, Christophe Fergeau <cfergeau at redhat.com>
wrote:

> Hey,
>
> On Tue, Jul 07, 2015 at 05:12:22PM +0300, Kirill Moizik wrote:
> > On Tue, Jul 7, 2015 at 3:51 PM, Christophe Fergeau <cfergeau at redhat.com>
> > wrote:
> >
> > > On Mon, Jul 06, 2015 at 08:59:06PM +0300, Kirill Moizik wrote:
> > > > we will spawn a separate thread to disconnect usb device, when
> finish we
> > > will update widget to allow further
> > > > usb redirection operations
> > > > 1) expose spice_usbredir_channel_connect_device_async function for
> > > asynchronous disconnect
> > > > 2) threads synchronization
> > >
> > > Honestly, moving non-trivial existing functions from running in the
> main
> > > thread to running in a separate thread with very little locking added,
> > > and no high level explanation as to why everything will be fine and
> > > nothing can be racy is very scary... One example below:
> > >
> >
> > I understand, I new to this project so i can't give you high level
> > explanation why there is no races. I am aware  there could be races, so i
> > spent time trying to find it. I tried few Os's on different hardware,
> with
> > and without spice/gtk debugging flags to change timing of flows to see if
> > it will fail, but l it worked for me. Also i was trying to add another
> > locks in suspicious places, but it led to deadlocks.  Do you have any
> ideas
> > how to prove quality of those patches ?
>
> Testing is unfortunately not a great way of proving something is never
> buggy, especially when it comes to race conditions. If something breaks
> during testing, then you know you have a bug, but if everything goes
> well, maybe you are just lucky. In this specific case, I'd try to do as
> little as possible in a thread, ideally just the libusb_open call which
> is what can block (btw, did we ever get feedback from the libusb
> maintainers if having libusb_open block was acceptable for them ?). You
> can use g_idle_add (for example), or
> g_simple_async_result_complete_in_idle() to make sure code runs in a
> 'known' context rather than in a random thread.


> Christophe
>

Yes, i understand that, it seems to me (may be i wrong) that nothing really
changes if only libusb_open will run in thread. It is the most time
consuming operation for usbdk backend, so it seem like same argumentation
can be applied in this case. May be we can think of some sort of quality
criteria and I will provide a test simulating  redirection flow with
gtk_test_widget_click? Just an idea. Anyway i will try to reduce amount of
code running in separate thread.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150708/d7c57a1e/attachment-0001.html>