[libusbx] Fix libusb_exit sometimes (race) deadlocking on exit (rhbz#985484)

Hans de Goede jwrdegoede at fedoraproject.org
Fri Jul 19 09:02:02 UTC 2013


commit 5ca63c4a7924edda2c5b8291f67faa7703378873
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Fri Jul 19 11:01:45 2013 +0200

    Fix libusb_exit sometimes (race) deadlocking on exit (rhbz#985484)

 ...-separate-lock-to-serialize-start-stop-vs.patch |  145 ++++++++++++++++++++
 libusbx.spec                                       |    7 +-
 2 files changed, 151 insertions(+), 1 deletions(-)
---
diff --git a/0001-linux-Use-a-separate-lock-to-serialize-start-stop-vs.patch b/0001-linux-Use-a-separate-lock-to-serialize-start-stop-vs.patch
new file mode 100644
index 0000000..c2e944f
--- /dev/null
+++ b/0001-linux-Use-a-separate-lock-to-serialize-start-stop-vs.patch
@@ -0,0 +1,145 @@
+From 4d0ae62ec9b80f49766104a1cae4f40b9b970bb9 Mon Sep 17 00:00:00 2001
+From: Hans de Goede <hdegoede at redhat.com>
+Date: Fri, 19 Jul 2013 10:47:07 +0200
+Subject: [PATCH 1/2] linux: Use a separate lock to serialize start/stop vs
+ hotplug events
+
+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.dickens at gmail.com>
+Signed-off-by: Hans de Goede <hdegoede at 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
+
diff --git a/libusbx.spec b/libusbx.spec
index 90e7146..adc7841 100644
--- a/libusbx.spec
+++ b/libusbx.spec
@@ -1,8 +1,9 @@
 Summary:        Library for accessing USB devices
 Name:           libusbx
 Version:        1.0.16
-Release:        1%{?dist}
+Release:        2%{?dist}
 Source0:        http://downloads.sourceforge.net/libusbx/libusbx-%{version}.tar.bz2
+Patch0:         0001-linux-Use-a-separate-lock-to-serialize-start-stop-vs.patch
 License:        LGPLv2+
 Group:          System Environment/Libraries
 URL:            http://sourceforge.net/apps/mediawiki/libusbx/
@@ -47,6 +48,7 @@ This package contains API documentation for %{name}.
 
 %prep
 %setup -q
+%patch0 -p1
 
 
 %build
@@ -80,6 +82,9 @@ rm $RPM_BUILD_ROOT%{_libdir}/*.la
 
 
 %changelog
+* Fri Jul 19 2013 Hans de Goede <hdegoede at redhat.com> - 1.0.16-2
+- Fix libusb_exit sometimes (race) deadlocking on exit (rhbz#985484)
+
 * Thu Jul 11 2013 Hans de Goede <hdegoede at redhat.com> - 1.0.16-1
 - New upstream 1.0.16 final release
 


More information about the scm-commits mailing list