From bec67d9814c56dcb0e557eed07dc941958645b9d Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Jul 30 2013 14:53:15 +0000 Subject: Fix another libusb_exit deadlock (rhbz#985484) --- 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 index c2e944f..b66abd4 100644 --- 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 @@ -1,6 +1,6 @@ -From 4d0ae62ec9b80f49766104a1cae4f40b9b970bb9 Mon Sep 17 00:00:00 2001 +From daf4c9fadaf8a49198c53c039bf78980dc251a4b Mon Sep 17 00:00:00 2001 From: Hans de Goede -Date: Fri, 19 Jul 2013 10:47:07 +0200 +Date: Tue, 30 Jul 2013 15:57:16 +0200 Subject: [PATCH 1/2] linux: Use a separate lock to serialize start/stop vs hotplug events @@ -17,77 +17,26 @@ Many thanks to Chris Dickens for figuring out the cause of this deadlock! CC: Chris Dickens Signed-off-by: Hans de Goede --- - 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(-) + libusb/os/linux_usbfs.c | 24 +++++++++++++++++------- + libusb/version_nano.h | 2 +- + 2 files changed, 18 insertions(+), 8 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 +index 09288af..90e23b7 100644 --- a/libusb/os/linux_usbfs.c +++ b/libusb/os/linux_usbfs.c -@@ -120,8 +120,8 @@ static int sysfs_has_descriptors = -1; +@@ -120,7 +120,9 @@ 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; ++/* Serialize scan-devices, event-thread, and poll */ + usbi_mutex_static_t linux_hotplug_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) +@@ -419,7 +421,7 @@ static int op_init(struct libusb_context *ctx) if (sysfs_has_descriptors) usbi_dbg("sysfs has complete descriptors"); @@ -96,7 +45,7 @@ index 09288af..1cdb1aa 100644 r = LIBUSB_SUCCESS; if (init_count == 0) { /* start up hotplug event handler */ -@@ -433,20 +433,20 @@ static int op_init(struct libusb_context *ctx) +@@ -433,20 +435,20 @@ static int op_init(struct libusb_context *ctx) linux_stop_event_monitor(); } else usbi_err(ctx, "error starting hotplug event monitor"); @@ -120,26 +69,35 @@ index 09288af..1cdb1aa 100644 } 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) +@@ -469,11 +471,19 @@ static int linux_stop_event_monitor(void) + + static int linux_scan_devices(struct libusb_context *ctx) + { ++ int ret; ++ ++ usbi_mutex_static_lock(&linux_hotplug_lock); ++ + #if defined(USE_UDEV) +- return linux_udev_scan_devices(ctx); ++ ret = linux_udev_scan_devices(ctx); + #else +- return linux_default_scan_devices(ctx); ++ ret = linux_default_scan_devices(ctx); + #endif ++ ++ usbi_mutex_static_unlock(&linux_hotplug_lock); ++ ++ return ret; + } --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); + static void op_hotplug_poll(void) diff --git a/libusb/version_nano.h b/libusb/version_nano.h -index 525cd7d..f84521e 100644 +index ebf41e1..34e26ff 100644 --- a/libusb/version_nano.h +++ b/libusb/version_nano.h @@ -1 +1 @@ -#define LIBUSB_NANO 10774 -+#define LIBUSB_NANO 10775 ++#define LIBUSB_NANO 10777 -- 1.8.3.1 diff --git a/0002-hotplug-Remove-use-of-pthread_cancel-from-linux_udev.patch b/0002-hotplug-Remove-use-of-pthread_cancel-from-linux_udev.patch new file mode 100644 index 0000000..5342fbe --- /dev/null +++ b/0002-hotplug-Remove-use-of-pthread_cancel-from-linux_udev.patch @@ -0,0 +1,141 @@ +From 852efb5a3e82de43cf6288e9904cb254ff636aa0 Mon Sep 17 00:00:00 2001 +From: Chris Dickens +Date: Sat, 20 Jul 2013 13:01:41 -0700 +Subject: [PATCH 2/2] hotplug: Remove use of pthread_cancel from linux_udev + +Using pthread_cancel() presents the opportunity for deadlock, so +use a control pipe to cause the event thread to gracefully exit. + +Signed-off-by: Hans de Goede +--- + libusb/os/linux_udev.c | 63 ++++++++++++++++++++++++++++++++++++++------------ + libusb/version_nano.h | 2 +- + 2 files changed, 49 insertions(+), 16 deletions(-) + +diff --git a/libusb/os/linux_udev.c b/libusb/os/linux_udev.c +index 5a2aadf..70f632d 100644 +--- a/libusb/os/linux_udev.c ++++ b/libusb/os/linux_udev.c +@@ -46,6 +46,7 @@ + /* udev context */ + static struct udev *udev_ctx = NULL; + static int udev_monitor_fd = -1; ++static int udev_control_pipe[2] = {-1, -1}; + static struct udev_monitor *udev_monitor = NULL; + static pthread_t linux_event_thread; + +@@ -95,14 +96,23 @@ int linux_udev_start_event_monitor(void) + goto err_free_monitor; + } + ++ r = usbi_pipe(udev_control_pipe); ++ if (r) { ++ usbi_err(NULL, "could not create udev control pipe"); ++ goto err_free_monitor; ++ } ++ + r = pthread_create(&linux_event_thread, NULL, linux_udev_event_thread_main, NULL); + if (r) { + usbi_err(NULL, "creating hotplug event thread (%d)", r); +- goto err_free_monitor; ++ goto err_close_pipe; + } + + return LIBUSB_SUCCESS; + ++err_close_pipe: ++ close(udev_control_pipe[0]); ++ close(udev_control_pipe[1]); + err_free_monitor: + udev_monitor_unref(udev_monitor); + udev_monitor = NULL; +@@ -115,14 +125,19 @@ err_free_ctx: + + int linux_udev_stop_event_monitor(void) + { ++ char dummy = 1; ++ int r; ++ + assert(udev_ctx != NULL); + assert(udev_monitor != NULL); + assert(udev_monitor_fd != -1); + +- /* Cancel the event thread. This is the only way to guarantee the +- thread exits since closing the monitor fd won't necessarily cause +- poll to return. */ +- pthread_cancel(linux_event_thread); ++ /* Write some dummy data to the control pipe and ++ * wait for the thread to exit */ ++ r = usbi_write(udev_control_pipe[1], &dummy, sizeof(dummy)); ++ if (r <= 0) { ++ usbi_warn(NULL, "udev control pipe signal failed"); ++ } + pthread_join(linux_event_thread, NULL); + + /* Release the udev monitor */ +@@ -134,27 +149,45 @@ int linux_udev_stop_event_monitor(void) + udev_unref(udev_ctx); + udev_ctx = NULL; + ++ /* close and reset control pipe */ ++ close(udev_control_pipe[0]); ++ close(udev_control_pipe[1]); ++ udev_control_pipe[0] = -1; ++ udev_control_pipe[1] = -1; ++ + return LIBUSB_SUCCESS; + } + + static void *linux_udev_event_thread_main(void *arg) + { ++ char dummy; ++ int r; + struct udev_device* udev_dev; +- struct pollfd fds = {.fd = udev_monitor_fd, +- .events = POLLIN}; ++ struct pollfd fds[] = { ++ {.fd = udev_control_pipe[0], ++ .events = POLLIN}, ++ {.fd = udev_monitor_fd, ++ .events = POLLIN}, ++ }; + + usbi_dbg("udev event thread entering."); + +- while (1 == poll(&fds, 1, -1)) { +- if (NULL == udev_monitor || POLLIN != fds.revents) { ++ while (poll(fds, 2, -1) >= 0) { ++ if (fds[0].revents & POLLIN) { ++ /* activity on control pipe, read the byte and exit */ ++ r = usbi_read(udev_control_pipe[0], &dummy, sizeof(dummy)); ++ if (r <= 0) { ++ usbi_warn(NULL, "udev control pipe read failed"); ++ } + break; + } +- +- usbi_mutex_static_lock(&linux_hotplug_lock); +- udev_dev = udev_monitor_receive_device(udev_monitor); +- if (udev_dev) +- udev_hotplug_event(udev_dev); +- usbi_mutex_static_unlock(&linux_hotplug_lock); ++ if (fds[1].revents & POLLIN) { ++ usbi_mutex_static_lock(&linux_hotplug_lock); ++ udev_dev = udev_monitor_receive_device(udev_monitor); ++ if (udev_dev) ++ udev_hotplug_event(udev_dev); ++ usbi_mutex_static_unlock(&linux_hotplug_lock); ++ } + } + + usbi_dbg("udev event thread exiting"); +diff --git a/libusb/version_nano.h b/libusb/version_nano.h +index 34e26ff..39ad7e3 100644 +--- a/libusb/version_nano.h ++++ b/libusb/version_nano.h +@@ -1 +1 @@ +-#define LIBUSB_NANO 10777 ++#define LIBUSB_NANO 10778 +-- +1.8.3.1 + diff --git a/libusbx.spec b/libusbx.spec index adc7841..b670e09 100644 --- a/libusbx.spec +++ b/libusbx.spec @@ -1,9 +1,10 @@ Summary: Library for accessing USB devices Name: libusbx Version: 1.0.16 -Release: 2%{?dist} +Release: 3%{?dist} Source0: http://downloads.sourceforge.net/libusbx/libusbx-%{version}.tar.bz2 Patch0: 0001-linux-Use-a-separate-lock-to-serialize-start-stop-vs.patch +Patch1: 0002-hotplug-Remove-use-of-pthread_cancel-from-linux_udev.patch License: LGPLv2+ Group: System Environment/Libraries URL: http://sourceforge.net/apps/mediawiki/libusbx/ @@ -49,6 +50,7 @@ This package contains API documentation for %{name}. %prep %setup -q %patch0 -p1 +%patch1 -p1 %build @@ -82,6 +84,9 @@ rm $RPM_BUILD_ROOT%{_libdir}/*.la %changelog +* Tue Jul 30 2013 Hans de Goede - 1.0.16-3 +- Fix another libusb_exit deadlock (rhbz#985484) + * Fri Jul 19 2013 Hans de Goede - 1.0.16-2 - Fix libusb_exit sometimes (race) deadlocking on exit (rhbz#985484)