core: Change event handling lock to traditional (non-recursive) type

The event handling lock was previously required to be of the recursive
type because the libusb_close() path requires the lock and may be
called by a thread that is handling events (e.g. from within a
transfer or hotplug callback). With commit 960a6e75, it is possible to
determine whether the current function is being called from an event
handling context, thus the recursive lock type is no longer necessary.

References:
* http://libusb.org/ticket/82
* 74282582cc879f091ad1d847411337bc3fa78a2b
* c775c2f43037cd235b65410583179195e25f9c4a

Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
[hdegoede@redhat.com: rebase on top of current master]
Signed-off-by: Hans de Goede <hdegoede@redhat.com>

diff --git a/libusb/core.c b/libusb/core.c
index 06342c8..2e3816c 100644 (file)
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -1340,8 +1340,6 @@ static void do_close(struct libusb_context *ctx,
        struct usbi_transfer *itransfer;
        struct usbi_transfer *tmp;
 
-       libusb_lock_events(ctx);
-
        /* remove any transfers in flight that are for this device */
        usbi_mutex_lock(&ctx->flying_transfers_lock);
 
@@ -1380,8 +1378,6 @@ static void do_close(struct libusb_context *ctx,
        }
        usbi_mutex_unlock(&ctx->flying_transfers_lock);
 
-       libusb_unlock_events(ctx);
-
        usbi_mutex_lock(&ctx->open_devs_lock);
        list_del(&dev_handle->list);
        usbi_mutex_unlock(&ctx->open_devs_lock);
@@ -1406,6 +1402,7 @@ static void do_close(struct libusb_context *ctx,
 void API_EXPORTED libusb_close(libusb_device_handle *dev_handle)
 {
        struct libusb_context *ctx;
+       int handling_events;
        int pending_events;
 
        if (!dev_handle)
@@ -1413,39 +1410,46 @@ void API_EXPORTED libusb_close(libusb_device_handle *dev_handle)
        usbi_dbg("");
 
        ctx = HANDLE_CTX(dev_handle);
+       handling_events = usbi_handling_events(ctx);
 
        /* Similarly to libusb_open(), we want to interrupt all event handlers
         * at this point. More importantly, we want to perform the actual close of
         * the device while holding the event handling lock (preventing any other
         * thread from doing event handling) because we will be removing a file
-        * descriptor from the polling loop. */
-
-       /* Record that we are closing a device.
-        * Only signal an event if there are no prior pending events. */
-       usbi_mutex_lock(&ctx->event_data_lock);
-       pending_events = usbi_pending_events(ctx);
-       ctx->device_close++;
-       if (!pending_events)
-               usbi_signal_event(ctx);
-       usbi_mutex_unlock(&ctx->event_data_lock);
-
-       /* take event handling lock */
-       libusb_lock_events(ctx);
+        * descriptor from the polling loop. If this is being called by the current
+        * event handler, we can bypass the interruption code because we already
+        * hold the event handling lock. */
+
+       if (!handling_events) {
+               /* Record that we are closing a device.
+                * Only signal an event if there are no prior pending events. */
+               usbi_mutex_lock(&ctx->event_data_lock);
+               pending_events = usbi_pending_events(ctx);
+               ctx->device_close++;
+               if (!pending_events)
+                       usbi_signal_event(ctx);
+               usbi_mutex_unlock(&ctx->event_data_lock);
+
+               /* take event handling lock */
+               libusb_lock_events(ctx);
+       }
 
        /* Close the device */
        do_close(ctx, dev_handle);
 
-       /* We're done with closing this device.
-        * Clear the event pipe if there are no further pending events. */
-       usbi_mutex_lock(&ctx->event_data_lock);
-       ctx->device_close--;
-       pending_events = usbi_pending_events(ctx);
-       if (!pending_events)
-               usbi_clear_event(ctx);
-       usbi_mutex_unlock(&ctx->event_data_lock);
-
-       /* Release event handling lock and wake up event waiters */
-       libusb_unlock_events(ctx);
+       if (!handling_events) {
+               /* We're done with closing this device.
+                * Clear the event pipe if there are no further pending events. */
+               usbi_mutex_lock(&ctx->event_data_lock);
+               ctx->device_close--;
+               pending_events = usbi_pending_events(ctx);
+               if (!pending_events)
+                       usbi_clear_event(ctx);
+               usbi_mutex_unlock(&ctx->event_data_lock);
+
+               /* Release event handling lock and wake up event waiters */
+               libusb_unlock_events(ctx);
+       }
 }
 
 /** \ingroup libusb_dev

diff --git a/libusb/io.c b/libusb/io.c
index 4d03b8b..3bd1675 100644 (file)
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1125,7 +1125,7 @@ int usbi_io_init(struct libusb_context *ctx)
        int r;
 
        usbi_mutex_init(&ctx->flying_transfers_lock);
-       usbi_mutex_init_recursive(&ctx->events_lock);
+       usbi_mutex_init(&ctx->events_lock);
        usbi_mutex_init(&ctx->event_waiters_lock);
        usbi_cond_init(&ctx->event_waiters_cond);
        usbi_mutex_init(&ctx->event_data_lock);

diff --git a/libusb/os/threads_posix.c b/libusb/os/threads_posix.c
index 3908907..a4f270b 100644 (file)
--- a/libusb/os/threads_posix.c
+++ b/libusb/os/threads_posix.c
@@ -37,28 +37,6 @@
 #include "threads_posix.h"
 #include "libusbi.h"
 
-int usbi_mutex_init_recursive(pthread_mutex_t *mutex)
-{
-       int err;
-       pthread_mutexattr_t attr;
-
-       err = pthread_mutexattr_init(&attr);
-       if (err != 0)
-               return err;
-
-       /* mutexattr_settype requires _GNU_SOURCE or _XOPEN_SOURCE >= 500 on Linux */
-       err = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
-       if (err != 0)
-               goto finish;
-
-       err = pthread_mutex_init(mutex, &attr);
-
-finish:
-       pthread_mutexattr_destroy(&attr);
-
-       return err;
-}
-
 int usbi_cond_timedwait(pthread_cond_t *cond,
        pthread_mutex_t *mutex, const struct timeval *tv)
 {

diff --git a/libusb/os/threads_posix.h b/libusb/os/threads_posix.h
index 2abb820..7ec70b1 100644 (file)
--- a/libusb/os/threads_posix.h
+++ b/libusb/os/threads_posix.h
@@ -50,8 +50,6 @@
 #define usbi_tls_key_set               pthread_setspecific
 #define usbi_tls_key_delete            pthread_key_delete
 
-int usbi_mutex_init_recursive(pthread_mutex_t *mutex);
-
 int usbi_cond_timedwait(pthread_cond_t *cond,
        pthread_mutex_t *mutex, const struct timeval *tv);
 

diff --git a/libusb/os/threads_windows.h b/libusb/os/threads_windows.h
index 8b7faec..e97ee78 100644 (file)
--- a/libusb/os/threads_windows.h
+++ b/libusb/os/threads_windows.h
@@ -71,9 +71,6 @@ void *usbi_tls_key_get(usbi_tls_key_t key);
 int usbi_tls_key_set(usbi_tls_key_t key, void *value);
 int usbi_tls_key_delete(usbi_tls_key_t key);
 
-// all Windows mutexes are recursive
-#define usbi_mutex_init_recursive      usbi_mutex_init
-
 int usbi_get_tid(void);
 
 #endif /* LIBUSB_THREADS_WINDOWS_H */

diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index e92bc71..74e5d3b 100644 (file)
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11128
+#define LIBUSB_NANO 11129