[Spice-devel] [PATCH spice-gtk 2/2] usb-device-manager: Add support for libusb hotplug API

[Hans de Goede]

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 at redhat.com> 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;
>> +}
>> +#endif
>
>
> Is 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