Using one lock for this is a bad idea, as we should not be holding any locks used by the hotplug thread when trying to stop otherwise the stop function may wait indefinetely in pthread_join, while the event-thread is waiting for the lock the caller of the stop function holds.
Using 2 separate locks for this should fix this deadlock, which has been reported here: https://bugzilla.redhat.com/show_bug.cgi?id=985484 Many thanks to Chris Dickens for figuring out the cause of this deadlock! CC: Chris Dickens <christopher.a.dick...@gmail.com> Signed-off-by: Hans de Goede <hdego...@redhat.com> --- libusb/os/linux_netlink.c | 5 ++++- libusb/os/linux_udev.c | 7 +++++++ libusb/os/linux_usbfs.c | 12 ++++++------ libusb/os/linux_usbfs.h | 2 -- libusb/version_nano.h | 2 +- 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/libusb/os/linux_netlink.c b/libusb/os/linux_netlink.c index 3a68f69..dc1ab96 100644 --- a/libusb/os/linux_netlink.c +++ b/libusb/os/linux_netlink.c @@ -47,7 +47,10 @@ static pthread_t libusb_linux_event_thread; static void *linux_netlink_event_thread_main(void *arg); -struct sockaddr_nl snl = { .nl_family=AF_NETLINK, .nl_groups=KERNEL }; +static struct sockaddr_nl snl = { .nl_family=AF_NETLINK, .nl_groups=KERNEL }; + +/* Serialize event-thread and poll */ +static usbi_mutex_static_t linux_hotplug_lock = USBI_MUTEX_INITIALIZER; int linux_netlink_start_event_monitor(void) { diff --git a/libusb/os/linux_udev.c b/libusb/os/linux_udev.c index 5a2aadf..f4edc10 100644 --- a/libusb/os/linux_udev.c +++ b/libusb/os/linux_udev.c @@ -52,6 +52,9 @@ static pthread_t linux_event_thread; static void udev_hotplug_event(struct udev_device* udev_dev); static void *linux_udev_event_thread_main(void *arg); +/* Serialize scan-devices, event-thread, and poll */ +static usbi_mutex_static_t linux_hotplug_lock = USBI_MUTEX_INITIALIZER; + int linux_udev_start_event_monitor(void) { int r; @@ -226,6 +229,8 @@ int linux_udev_scan_devices(struct libusb_context *ctx) assert(udev_ctx != NULL); + usbi_mutex_static_lock(&linux_hotplug_lock); + enumerator = udev_enumerate_new(udev_ctx); if (NULL == enumerator) { usbi_err(ctx, "error creating udev enumerator"); @@ -254,6 +259,8 @@ int linux_udev_scan_devices(struct libusb_context *ctx) udev_enumerate_unref(enumerator); + usbi_mutex_static_unlock(&linux_hotplug_lock); + return LIBUSB_SUCCESS; } diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c index 09288af..1cdb1aa 100644 --- a/libusb/os/linux_usbfs.c +++ b/libusb/os/linux_usbfs.c @@ -120,8 +120,8 @@ static int sysfs_has_descriptors = -1; /* how many times have we initted (and not exited) ? */ static volatile int init_count = 0; -/* Serialize hotplug start/stop, scan-devices, event-thread, and poll */ -usbi_mutex_static_t linux_hotplug_lock = USBI_MUTEX_INITIALIZER; +/* Serialize hotplug start/stop */ +usbi_mutex_static_t linux_hotplug_startstop_lock = USBI_MUTEX_INITIALIZER; static int linux_start_event_monitor(void); static int linux_stop_event_monitor(void); @@ -419,7 +419,7 @@ static int op_init(struct libusb_context *ctx) if (sysfs_has_descriptors) usbi_dbg("sysfs has complete descriptors"); - usbi_mutex_static_lock(&linux_hotplug_lock); + usbi_mutex_static_lock(&linux_hotplug_startstop_lock); r = LIBUSB_SUCCESS; if (init_count == 0) { /* start up hotplug event handler */ @@ -433,20 +433,20 @@ static int op_init(struct libusb_context *ctx) linux_stop_event_monitor(); } else usbi_err(ctx, "error starting hotplug event monitor"); - usbi_mutex_static_unlock(&linux_hotplug_lock); + usbi_mutex_static_unlock(&linux_hotplug_startstop_lock); return r; } static void op_exit(void) { - usbi_mutex_static_lock(&linux_hotplug_lock); + usbi_mutex_static_lock(&linux_hotplug_startstop_lock); assert(init_count != 0); if (!--init_count) { /* tear down event handler */ (void)linux_stop_event_monitor(); } - usbi_mutex_static_unlock(&linux_hotplug_lock); + usbi_mutex_static_unlock(&linux_hotplug_startstop_lock); } static int linux_start_event_monitor(void) diff --git a/libusb/os/linux_usbfs.h b/libusb/os/linux_usbfs.h index 499bab7..17e6e0f 100644 --- a/libusb/os/linux_usbfs.h +++ b/libusb/os/linux_usbfs.h @@ -156,8 +156,6 @@ struct usbfs_disconnect_claim { #define IOCTL_USBFS_GET_CAPABILITIES _IOR('U', 26, __u32) #define IOCTL_USBFS_DISCONNECT_CLAIM _IOR('U', 27, struct usbfs_disconnect_claim) -extern usbi_mutex_static_t linux_hotplug_lock; - #if defined(HAVE_LIBUDEV) int linux_udev_start_event_monitor(void); int linux_udev_stop_event_monitor(void); diff --git a/libusb/version_nano.h b/libusb/version_nano.h index 525cd7d..f84521e 100644 --- a/libusb/version_nano.h +++ b/libusb/version_nano.h @@ -1 +1 @@ -#define LIBUSB_NANO 10774 +#define LIBUSB_NANO 10775 -- 1.8.3.1 ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk _______________________________________________ libusbx-devel mailing list libusbx-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libusbx-devel
