On Mon, Jan 02, 2012 at 03:01:39PM +0100, Hans de Goede wrote: > Hi, > > On 01/02/2012 01:59 PM, Christophe Fergeau wrote: > >On Sat, Dec 24, 2011 at 10:27:40AM +0100, Hans de Goede wrote: > >>On 12/23/2011 06:26 PM, Christophe Fergeau wrote: > >>>is there > >>>something preventing the check on priv->state from racing with code running > >>>in the main thread? > >> > >>Oh, good one I must admit I did not think about this. I believe this is not > >>a > >>problem though: > >> > >>The write flush callback causes libusbredirhost to write out writes queued > >>inside libusbredirhost (so in essence they are moved from a libusbredirhost > >>internal queue to the channel's xmit queue). > >> > >>There are 2 cases where the state can change, the channel going up (so going > >>to state == CONNECTED) and the channel going down. Going down is not really > >>interesting, because once the channel is disconnected it does not matter if > >>writes are buffered inside libusbredirhost or inside the channel's xmit > >>queue, as the channel is tore down they will get discarded either way. > > > >I looked a bit more at the code, and I'm still worried by > >spice_usbredir_channel_disconnect running while > >usbredir_write_flush_callback executes, ie: > > > >flush_callback checks priv->state != STATE_CONNECTED > > > >disconnect is called from the main thread (eg from dispose) and calls > >usbredirhost_close(priv->host) which frees priv->host and then is preempted > > > >flush_callback resumes and runs usbredirhost_write_guest_data(priv->host); > >which has been freed but not set to NULL. > > > > > >I'm not exactly sure at what point the flush callback stops being called by > >usbredir, so I prefer to mention this here and get explained why this can't > >happen :) > > This cannot happen because usbredirhost_close() calls libusb_close on the > device > handle for the device for this channel, and this will cleanup (kill) any > not yet completed transfers. libusb guarantees that after a libusb_close() no > packet complete callbacks will get called for the closed device. So after > libusb_close() has completed the usbredir_write_flush_callback will never get > called for this channel again.
Yes, but in the example I gave, the flush callback starts executing before any call to libusb_close, and then got preempted by the main thread calling _disconnect. There will be no more invokations of the flush callback after the call to libusb_close, but the flush callback execution that got preempted won't magically get cancelled, will it? Christophe
pgpFy1Frbz5Ys.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/spice-devel
