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
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -2123,21 +2123,29 @@
/* complete any pending transfers */
if (ctx->event_flags & USBI_EVENT_TRANSFER_COMPLETED) {
- 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);
+ struct usbi_transfer *itransfer, *tmp;
+ struct list_head completed_transfers;
+ assert(!list_empty(&ctx->completed_transfers));
+ 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
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -203,8 +203,10 @@
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 @@
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 @@
#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
--- a/libusb/os/linux_usbfs.c
+++ b/libusb/os/linux_usbfs.c
@@ -2654,6 +2654,7 @@
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 @@
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
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11552
+#define LIBUSB_NANO 11553
