Hi, Thanks for the review! On 07/05/2013 01:46 AM, Marc-André Lureau wrote:
Hi On Thu, Jul 4, 2013 at 5:13 PM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:+/* Can be called from both the main-thread as well as the event_thread */ +static int spice_usb_device_manager_hotplug_cb(libusb_context *ctx, + libusb_device *device, + libusb_hotplug_event event, + void *user_data) +{ + struct hotplug_idle_cb_args *args = g_malloc(sizeof(*args)); + + args->self = user_data; + args->device = libusb_ref_device(device); + args->event = event; + g_idle_add(spice_usb_device_manager_hotplug_idle_cb, args); + return 0; +} +#endifIs there a good reason not to keep a reference on the idle and data, and cancel it if self is disposed?
There can be (and will be during initial enumeration) multiple spice_usb_device_manager_hotplug_idle_cb pending. So we would need a list for this, not un-doable, but not my preferred solution.
Or add a ref of self (if that's really short-lived and safe)?
Yes, good point, the code should ref self. Note that spice_usb_device_manager_hotplug_cb can be called while finalize is running. So we should move the stopping of the event-thread to dispose. About this being safe, I've just checked the gobject code, and glib will do the right thing when we stop the thread from dispose. The "final" g_object_unref does: -call dispose (because ref count becomes 0) -check if ref-count is still 0, otherwise bail. -call finalize So if an idle-callback with a ref gets added before dispose stops the thread, then g_object_unref will stop after calling dispose. Then when the idle-callback runs, it will call g_object_unref, and that one will be the final unref and will call finalize and free the memory. I'll respin the patch to add the refs and move the thread stopping to dispose. Regards, Hans _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel