On Wed, Apr 23, 2014 at 11:15 PM, Jonathon Jongsma <jjongsma@xxxxxxxxxx> wrote:
Hm, I agree that the existing code does look very suspicious, but in general I think that ensuring that a ref is held for the duration of the async operation is probably a good idea. From a very cursory investigation it looks like the spice_usb_device_manager_remove_dev() call path could be problematic. This ends up calling spice_win_usb_driver_uninstall() and then removing the device (via g_ptr_array_remove()), which could end up dropping the final ref on the object. Then you'd have a dead object in the uninstall_cb callback. Granted, the existing code doesn't appear to be safe in this scenario either...
I agree with you that you must be careful to hold a ref to your object during async. But the current ref/unref is useless and suspicious. So it should be removed in its current form. Feel free to improve the code for most safety, this is not my goal with this patch.
--
Marc-André Lureau
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel