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

[Christophe Fergeau]

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
-------------- 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/20150708/5e4a9627/attachment.sig>