Guard against getting stuck while handling events

Alba Mendez (user mildsunrise) reports that a thread can get stuck at a
specific point while handling events, thus preventing other events from
being handled. This change addresses two such points in the code:

  1) When processing completed transfers in handle_event_trigger(), the
     function wll loop for as long as the completed_transfers list is
     not empty. Since the event data lock is released and reacquired for
     each completed transfer, it is possible for the completed_transfers
     list to never be emptied. Address this by cutting the list and only
     process the transfers that have completed at that point in time.

  2) When processing events, the Linux backend will reap transfers for
     each device that indicates activity until the reap fails (either
     because there are no completed transfers or some other error
     occurs). It is possible for transfers to be reaped at a rate slower
     than that at which they are completing, thus preventing the loop
     from exiting. Address this by limiting the number of transfers
     reaped for each device to 25 for every call to the Linux backend's
     handle_events() function.

Closes #780

Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>

diff --git a/libusb/io.c b/libusb/io.c
index 272177e..9db322b 100644 (file)
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -2123,21 +2123,29 @@ static int handle_event_trigger(struct libusb_context *ctx)
 
        /* complete any pending transfers */
        if (ctx->event_flags & USBI_EVENT_TRANSFER_COMPLETED) {
+               struct usbi_transfer *itransfer, *tmp;
+               struct list_head completed_transfers;
+
                assert(!list_empty(&ctx->completed_transfers));
-               while (r == 0 && !list_empty(&ctx->completed_transfers)) {
-                       struct usbi_transfer *itransfer =
-                               list_first_entry(&ctx->completed_transfers, struct usbi_transfer, completed_list);
+               list_cut(&completed_transfers, &ctx->completed_transfers);
+               usbi_mutex_unlock(&ctx->event_data_lock);
 
+               __for_each_transfer_safe(&completed_transfers, itransfer, tmp) {
                        list_del(&itransfer->completed_list);
-                       usbi_mutex_unlock(&ctx->event_data_lock);
                        r = usbi_backend.handle_transfer_completion(itransfer);
-                       if (r)
+                       if (r) {
                                usbi_err(ctx, "backend handle_transfer_completion failed with error %d", r);
-                       usbi_mutex_lock(&ctx->event_data_lock);
+                               break;
+                       }
                }
 
-               if (list_empty(&ctx->completed_transfers))
+               usbi_mutex_lock(&ctx->event_data_lock);
+               if (!list_empty(&completed_transfers)) {
+                       /* an error occurred, put the remaining transfers back on the list */
+                       list_splice_front(&completed_transfers, &ctx->completed_transfers);
+               } else if (list_empty(&ctx->completed_transfers)) {
                        ctx->event_flags &= ~USBI_EVENT_TRANSFER_COMPLETED;
+               }
        }
 
        /* if no further pending events, clear the event */

diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index d0e57e4..da45121 100644 (file)
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -203,8 +203,10 @@ static inline void list_del(struct list_head *entry)
 
 static inline void list_cut(struct list_head *list, struct list_head *head)
 {
-       if (list_empty(head))
+       if (list_empty(head)) {
+               list_init(list);
                return;
+       }
 
        list->next = head->next;
        list->next->prev = list;
@@ -214,6 +216,13 @@ static inline void list_cut(struct list_head *list, struct list_head *head)
        list_init(head);
 }
 
+static inline void list_splice_front(struct list_head *list, struct list_head *head)
+{
+       list->next->prev = head;
+       list->prev->next = head->next;
+       head->next = list->next;
+}
+
 static inline void *usbi_reallocf(void *ptr, size_t size)
 {
        void *ret = realloc(ptr, size);
@@ -1306,11 +1315,17 @@ extern const struct usbi_os_backend usbi_backend;
 #define for_each_open_device(ctx, h) \
        for_each_helper(h, &(ctx)->open_devs, struct libusb_device_handle)
 
+#define __for_each_transfer(list, t) \
+       for_each_helper(t, (list), struct usbi_transfer)
+
 #define for_each_transfer(ctx, t) \
-       for_each_helper(t, &(ctx)->flying_transfers, struct usbi_transfer)
+       __for_each_transfer(&(ctx)->flying_transfers, t)
+
+#define __for_each_transfer_safe(list, t, n) \
+       for_each_safe_helper(t, n, (list), struct usbi_transfer)
 
 #define for_each_transfer_safe(ctx, t, n) \
-       for_each_safe_helper(t, n, &(ctx)->flying_transfers, struct usbi_transfer)
+       __for_each_transfer_safe(&(ctx)->flying_transfers, t, n)
 
 #define for_each_event_source(ctx, e) \
        for_each_helper(e, &(ctx)->event_sources, struct usbi_event_source)

diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
index 27bed33..f5c92c2 100644 (file)
--- a/libusb/os/linux_usbfs.c
+++ b/libusb/os/linux_usbfs.c
@@ -2654,6 +2654,7 @@ static int op_handle_events(struct libusb_context *ctx,
                struct pollfd *pollfd = &fds[n];
                struct libusb_device_handle *handle;
                struct linux_device_handle_priv *hpriv = NULL;
+               int reap_count;
 
                if (!pollfd->revents)
                        continue;
@@ -2696,9 +2697,11 @@ static int op_handle_events(struct libusb_context *ctx,
                        continue;
                }
 
+               reap_count = 0;
                do {
                        r = reap_for_handle(handle);
-               } while (r == 0);
+               } while (r == 0 && ++reap_count <= 25);
+
                if (r == 1 || r == LIBUSB_ERROR_NO_DEVICE)
                        continue;
                else if (r < 0)

diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index e9e0512..a2be5c2 100644 (file)
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11552
+#define LIBUSB_NANO 11553