<div dir="ltr"> <div class="gmail_extra"><div class="gmail_quote">On Wed, Jul 8, 2015 at 12:07 PM, Christophe Fergeau <<a href="mailto:cfergeau@redhat.com" target="_blank">cfergeau@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">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 <<a href="mailto:cfergeau@redhat.com" target="_blank">cfergeau@redhat.com</a>> > 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. </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> Christophe </blockquote></div> </div><div class="gmail_extra">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. </div></div>