Skip to content

Commit

Permalink
Guard against getting stuck while handling events
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
dickens committed Sep 13, 2020
1 parent 89b810e commit 006ca0f
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 12 deletions.
22 changes: 15 additions & 7 deletions libusb/io.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
21 changes: 18 additions & 3 deletions libusb/libusbi.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion libusb/os/linux_usbfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion libusb/version_nano.h
Original file line number Diff line number Diff line change
@@ -1 +1 @@
#define LIBUSB_NANO 11552
#define LIBUSB_NANO 11553

0 comments on commit 006ca0f

Please sign in to comment.