[Spice-devel] [PATCH spice-gtk 04/10] usbredir: Use new libusbredirhost write flush callback

[Christophe Fergeau]

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 :)

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20120102/b2cf5f2a/attachment.pgp>