Skip to content

Commit

Permalink
context: Fix hotplug handling and signal emission during enumerate
Browse files Browse the repository at this point in the history
Initial device enumeration may happen in a random order. This causes
issues if someone calls g_usb_device_get_parent from the "device-added"
signal handler. The problem is. that this will cause an infinite
recursion as GUsb tries to enumerate again, which results in the device
to be added. Also, the order of devices is random and resolving the
parent requires the full list of devices. As such, we should delay
emission until the internal array has been filled.

To fix this, emit the signals from g_usb_context_enumerate.

Also, while changing this, it became obvious that libusb hotplug callback
is updating priv->devices from the libusb thread, only pushing it to an
idle handler when the signal is emitted. Doing this is unsafe if hotplug
events arrive during enumeration. Push libusb event into the mainloop
immediately in order to process it.

Doing means that any libusb hotplug events arriving during
g_usb_context_enumerate will be handled after the function has returned.
Making all possible corner cases behave correctly.
  • Loading branch information
Benjamin Berg authored and hughsie committed Oct 5, 2021
1 parent a451c0f commit 4b52b0c
Showing 1 changed file with 56 additions and 50 deletions.
106 changes: 56 additions & 50 deletions gusb/gusb-context.c
Original file line number Diff line number Diff line change
Expand Up @@ -242,59 +242,26 @@ g_usb_context_class_init (GUsbContextClass *klass)
G_USB_TYPE_DEVICE);
}

typedef struct {
GUsbContext *context;
GUsbDevice *device;
guint signal_id;
} GUsbContextIdleHelper;

static void
g_usb_context_idle_helper_free (GUsbContextIdleHelper *helper)
{
g_object_unref (helper->context);
g_object_unref (helper->device);
g_free (helper);
}

static gboolean
g_usb_context_idle_signal_cb (gpointer user_data)
{
GUsbContextIdleHelper *helper = (GUsbContextIdleHelper *) user_data;
g_signal_emit (helper->context, signals[helper->signal_id], 0, helper->device);
g_usb_context_idle_helper_free (helper);
return FALSE;
}

static void
g_usb_context_emit_device_add (GUsbContext *context,
GUsbDevice *device)
{
GUsbContextIdleHelper *helper;
helper = g_new0 (GUsbContextIdleHelper, 1);
helper->context = g_object_ref (context);
helper->device = g_object_ref (device);
helper->signal_id = DEVICE_ADDED_SIGNAL;
if (!context->priv->done_enumerate) {
g_usb_context_idle_signal_cb (helper);
/* emitted directly by g_usb_context_enumerate */
if (!context->priv->done_enumerate)
return;
}
g_idle_add (g_usb_context_idle_signal_cb, helper);

g_signal_emit (context, signals[DEVICE_ADDED_SIGNAL], 0, device);
}

static void
g_usb_context_emit_device_remove (GUsbContext *context,
GUsbDevice *device)
{
GUsbContextIdleHelper *helper;
helper = g_new0 (GUsbContextIdleHelper, 1);
helper->context = g_object_ref (context);
helper->device = g_object_ref (device);
helper->signal_id = DEVICE_REMOVED_SIGNAL;
if (!context->priv->done_enumerate) {
g_usb_context_idle_signal_cb (helper);
/* should not happen, if it does we would not fire any signal */
if (!context->priv->done_enumerate)
return;
}
g_idle_add (g_usb_context_idle_signal_cb, helper);

g_signal_emit (context, signals[DEVICE_REMOVED_SIGNAL], 0, device);
}

static void
Expand Down Expand Up @@ -396,24 +363,56 @@ g_usb_context_remove_device (GUsbContext *context,
g_object_unref (device);
}

static int
g_usb_context_hotplug_cb (struct libusb_context *ctx,
struct libusb_device *dev,
libusb_hotplug_event event,
void *user_data)
typedef struct {
GUsbContext *context;
libusb_device *dev;
libusb_hotplug_event event;
} GUsbContextIdleHelper;

static void
g_usb_context_idle_helper_free (GUsbContextIdleHelper *helper)
{
GUsbContext *context = G_USB_CONTEXT (user_data);
g_object_unref (helper->context);
libusb_unref_device (helper->dev);
g_free (helper);
}

switch (event) {
static gboolean
g_usb_context_idle_hotplug_cb (gpointer user_data)
{
GUsbContextIdleHelper *helper = (GUsbContextIdleHelper *) user_data;

switch (helper->event) {
case LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED:
g_usb_context_add_device (context, dev);
g_usb_context_add_device (helper->context, helper->dev);
break;
case LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT:
g_usb_context_remove_device (context, dev);
g_usb_context_remove_device (helper->context, helper->dev);
break;
default:
break;
}

g_usb_context_idle_helper_free (helper);
return FALSE;
}

static int
g_usb_context_hotplug_cb (struct libusb_context *ctx,
struct libusb_device *dev,
libusb_hotplug_event event,
void *user_data)
{
GUsbContext *context = G_USB_CONTEXT (user_data);
GUsbContextIdleHelper *helper;

helper = g_new0 (GUsbContextIdleHelper, 1);
helper->context = context;
helper->dev = libusb_ref_device (dev);
helper->event = event;

g_idle_add (g_usb_context_idle_hotplug_cb, helper);

return 0;
}

Expand Down Expand Up @@ -538,6 +537,13 @@ g_usb_context_enumerate (GUsbContext *context)
context);
}
priv->done_enumerate = TRUE;

/* emit device-added signals before returning */
for (guint i = 0; i < priv->devices->len; i++)
g_signal_emit (context, signals[DEVICE_ADDED_SIGNAL], 0,
g_ptr_array_index (priv->devices, i));

/* any queued up hotplug events are queued as idle handlers */
}

/**
Expand Down

0 comments on commit 4b52b0c

Please sign in to comment.