Hi,
On 01/02/2012 03:10 PM, Christophe Fergeau wrote:
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?
Ah, sorry, I thought of that and checked the actual libusb code before sending
of the mail where I claimed this was not an issue, but I guess was not clear:
On device close libusb kills any pending transfer *and* waits for any running
transfer
complete callbacks to finish, it does so in a thread safe manner.
libusb has something called an event handler lock, which is held when the
callback
is called, and libusb_close takes this lock thus ensuring that any callbacks
already running, when libusb_close gets called, have completed before
libusb_close
completes.
Regards,
Hans
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
