Re: [PATCH v2 6/6] Make usb redirection stop and surprise removal flows asynchronous

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Wed, Jul 8, 2015 at 12:07 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> 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@xxxxxxxxxx>
> 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. 
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]