From d246f2e9880f8969b031775c1c0abff1bec2ce3f Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Wed, 10 Aug 2016 12:17:00 +0200
Subject: [PATCH 01/11] Revert "io: Fix race condition in handle_timeout()"
This reverts commit bd8d5b5019b72b2dc2d074d96c9992e2f6e7e0b7.
Chris Dickens and me have been working on a patch-set refactoring
the transfer flag handling which fixes this differently. Revert
this commit so that the refactoring changes can be merged cleanly.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
libusb/io.c | 57 ++++++++++++++++++---------------------------------
libusb/version_nano.h | 2 +-
2 files changed, 21 insertions(+), 38 deletions(-)
diff --git a/libusb/io.c b/libusb/io.c
index 8944461..4d03b8b 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1530,34 +1530,6 @@ out:
return r;
}
-static int cancel_transfer_locked(struct libusb_transfer *transfer)
-{
- struct usbi_transfer *itransfer =
- LIBUSB_TRANSFER_TO_USBI_TRANSFER(transfer);
- int r;
- if (!(itransfer->flags & USBI_TRANSFER_IN_FLIGHT)
- || (itransfer->flags & USBI_TRANSFER_CANCELLING)) {
- return LIBUSB_ERROR_NOT_FOUND;
- }
-
- r = usbi_backend->cancel_transfer(itransfer);
- if (r < 0) {
- if (r != LIBUSB_ERROR_NOT_FOUND &&
- r != LIBUSB_ERROR_NO_DEVICE)
- usbi_err(TRANSFER_CTX(transfer),
- "cancel transfer failed error %d", r);
- else
- usbi_dbg("cancel transfer failed error %d", r);
-
- if (r == LIBUSB_ERROR_NO_DEVICE)
- itransfer->flags |= USBI_TRANSFER_DEVICE_DISAPPEARED;
- }
-
- itransfer->flags |= USBI_TRANSFER_CANCELLING;
-
- return r;
-}
-
/** \ingroup libusb_asyncio
* Asynchronously cancel a previously submitted transfer.
* This function returns immediately, but this does not indicate cancellation
@@ -1581,9 +1553,27 @@ int API_EXPORTED libusb_cancel_transfer(struct libusb_transfer *transfer)
usbi_dbg("transfer %p", transfer );
usbi_mutex_lock(&itransfer->lock);
usbi_mutex_lock(&itransfer->flags_lock);
+ if (!(itransfer->flags & USBI_TRANSFER_IN_FLIGHT)
+ || (itransfer->flags & USBI_TRANSFER_CANCELLING)) {
+ r = LIBUSB_ERROR_NOT_FOUND;
+ goto out;
+ }
+ r = usbi_backend->cancel_transfer(itransfer);
+ if (r < 0) {
+ if (r != LIBUSB_ERROR_NOT_FOUND &&
+ r != LIBUSB_ERROR_NO_DEVICE)
+ usbi_err(TRANSFER_CTX(transfer),
+ "cancel transfer failed error %d", r);
+ else
+ usbi_dbg("cancel transfer failed error %d", r);
- r = cancel_transfer_locked(transfer);
+ if (r == LIBUSB_ERROR_NO_DEVICE)
+ itransfer->flags |= USBI_TRANSFER_DEVICE_DISAPPEARED;
+ }
+
+ itransfer->flags |= USBI_TRANSFER_CANCELLING;
+out:
usbi_mutex_unlock(&itransfer->flags_lock);
usbi_mutex_unlock(&itransfer->lock);
return r;
@@ -1977,20 +1967,13 @@ static void handle_timeout(struct usbi_transfer *itransfer)
USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
int r;
- usbi_mutex_lock(&itransfer->lock);
- usbi_mutex_lock(&itransfer->flags_lock);
-
itransfer->flags |= USBI_TRANSFER_TIMEOUT_HANDLED;
- r = cancel_transfer_locked(transfer);
-
+ r = libusb_cancel_transfer(transfer);
if (r == 0)
itransfer->flags |= USBI_TRANSFER_TIMED_OUT;
else
usbi_warn(TRANSFER_CTX(transfer),
"async cancel failed %d errno=%d", r, errno);
-
- usbi_mutex_unlock(&itransfer->flags_lock);
- usbi_mutex_unlock(&itransfer->lock);
}
static int handle_timeouts_locked(struct libusb_context *ctx)
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index a806b19..e92bc71 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11127
+#define LIBUSB_NANO 11128
--
2.7.4
From cdea26fe7efeac3c41e2668d32c4ef7873298a9a Mon Sep 17 00:00:00 2001
From: Chris Dickens <christopher.a.dickens@gmail.com>
Date: Mon, 26 Oct 2015 14:11:53 +0100
Subject: [PATCH 02/11] 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>
---
libusb/core.c | 60 ++++++++++++++++++++++++---------------------
libusb/io.c | 2 +-
libusb/os/threads_posix.c | 22 -----------------
libusb/os/threads_posix.h | 2 --
libusb/os/threads_windows.h | 3 ---
libusb/version_nano.h | 2 +-
6 files changed, 34 insertions(+), 57 deletions(-)
diff --git a/libusb/core.c b/libusb/core.c
index 06342c8..2e3816c 100644
--- 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
--- 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
--- 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
--- 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
--- 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
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11128
+#define LIBUSB_NANO 11129
--
2.7.4
From 3253bd9d5d583ce4a9b32d13be3bf038accee977 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Mon, 6 Jun 2016 16:40:54 +0200
Subject: [PATCH 03/11] wince: Do not differ between cancel / timeout ourselves
(itransfer->flags & USBI_TRANSFER_TIMED_OUT) is already checked by
usbi_handle_transfer_cancellation(), which wince_transfer_callback()
will call when status == LIBUSB_TRANSFER_CANCELLED. Leave this up
to the core, so that future changes to timeout handling do no break
wince.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Note: untested
---
libusb/os/wince_usb.c | 9 ++-------
libusb/version_nano.h | 2 +-
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/libusb/os/wince_usb.c b/libusb/os/wince_usb.c
index 85c584b..89b5e65 100644
--- a/libusb/os/wince_usb.c
+++ b/libusb/os/wince_usb.c
@@ -720,13 +720,8 @@ static void wince_transfer_callback(
status = LIBUSB_TRANSFER_TIMED_OUT;
break;
case ERROR_OPERATION_ABORTED:
- if (itransfer->flags & USBI_TRANSFER_TIMED_OUT) {
- usbi_dbg("detected timeout");
- status = LIBUSB_TRANSFER_TIMED_OUT;
- } else {
- usbi_dbg("detected operation aborted");
- status = LIBUSB_TRANSFER_CANCELLED;
- }
+ usbi_dbg("detected operation aborted");
+ status = LIBUSB_TRANSFER_CANCELLED;
break;
default:
usbi_err(ITRANSFER_CTX(itransfer), "detected I/O error: %s", windows_error_str(io_result));
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index 74e5d3b..335ae07 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11129
+#define LIBUSB_NANO 11130
--
2.7.4
From 916c1f49a20cbd151a8972bc21582dc311ed1124 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Mon, 6 Jun 2016 16:43:18 +0200
Subject: [PATCH 04/11] winnt: Do not differ between cancel / timeout ourselves
(itransfer->flags & USBI_TRANSFER_TIMED_OUT) is already checked by
usbi_handle_transfer_cancellation(), make windows_transfer_callback()
call usbi_handle_transfer_cancellation() when
status == LIBUSB_TRANSFER_CANCELLED like all other os backends do, and
leave USBI_TRANSFER_TIMED_OUT handling up to the core, so that future
changes to timeout handling do no break winnt.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Note: untested
---
libusb/os/windows_nt_common.c | 16 +++++++---------
libusb/version_nano.h | 2 +-
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/libusb/os/windows_nt_common.c b/libusb/os/windows_nt_common.c
index 93e6278..68eb4a7 100644
--- a/libusb/os/windows_nt_common.c
+++ b/libusb/os/windows_nt_common.c
@@ -501,14 +501,9 @@ static void windows_transfer_callback(struct usbi_transfer *itransfer, uint32_t
istatus = windows_copy_transfer_data(itransfer, io_size);
if (istatus != LIBUSB_TRANSFER_COMPLETED)
usbi_dbg("Failed to copy partial data in aborted operation: %d", istatus);
- if (itransfer->flags & USBI_TRANSFER_TIMED_OUT) {
- usbi_dbg("detected timeout");
- status = LIBUSB_TRANSFER_TIMED_OUT;
- }
- else {
- usbi_dbg("detected operation aborted");
- status = LIBUSB_TRANSFER_CANCELLED;
- }
+
+ usbi_dbg("detected operation aborted");
+ status = LIBUSB_TRANSFER_CANCELLED;
break;
default:
usbi_err(ITRANSFER_CTX(itransfer), "detected I/O error %u: %s", io_result, windows_error_str(io_result));
@@ -516,7 +511,10 @@ static void windows_transfer_callback(struct usbi_transfer *itransfer, uint32_t
break;
}
windows_clear_transfer_priv(itransfer); // Cancel polling
- usbi_handle_transfer_completion(itransfer, (enum libusb_transfer_status)status);
+ if (status == LIBUSB_TRANSFER_CANCELLED)
+ usbi_handle_transfer_cancellation(itransfer);
+ else
+ usbi_handle_transfer_completion(itransfer, (enum libusb_transfer_status)status);
}
void windows_handle_callback(struct usbi_transfer *itransfer, uint32_t io_result, uint32_t io_size)
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index 335ae07..3fdb625 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11130
+#define LIBUSB_NANO 11131
--
2.7.4
From 5db9f1f0adb5556457859f329217fb031f2f5fb4 Mon Sep 17 00:00:00 2001
From: Chris Dickens <christopher.a.dickens@gmail.com>
Date: Mon, 26 Oct 2015 14:18:33 +0100
Subject: [PATCH 05/11] core: Refactor code related to transfer flags and
timeout handling
Commit a886bb02 sped up the library a bit by removing the serialization
of transfer submission with respect to the flying_transfers list, but
it introduced two separate issues.
1) A deadlock scenario is possible given the following sequence:
- Thread A submits transfer with very short timeout (say 1ms)
-> takes transfer->lock
-> adds transfer to flying_transfers list and arms timerfd
-> actually calls backend to submit transfer, but it fails
<context switch>
- Thread B is doing event handling and sees the timerfd trigger
-> takes ctx->flying_transfers_lock
-> finds the transfer above on the list
-> calls libusb_cancel_transfer() for this transfer
--> takes transfer->lock
<context switch>
- Thread A sees the transfer failed to submit
-> removes transfer from flying_transfers list
--> takes ctx->flying_transfers_lock (still holding transfer->lock)
** DEADLOCK **
2) The transfer state flags (e.g. submitting, in-flight) were protected
by transfer->flags_lock, but the timeout-related flags were OR'ed in
during timeout handling operations outside of the lock. This leads to
the possibility that transfer state might get overwritten.
This change corrects these issues and simplifies the transfer submission
code a bit by separating the state and timeout flags into their own flag
variables. The state flags are protected by the transfer lock. The timeout
flags are protected by the flying_transfers_lock.
The transfer submission code sheds some weight because it no longer needs
to worry about the timing of events that modify the transfer state flags.
These flags are always viewed and modified under the protection of the
transfer lock. Since libusb_submit_transfer() holds the transfer lock for
the entire duration of the operation, the other code paths that would
possibly touch the transfer (e.g. usbi_handle_disconnect() and
usbi_handle_transfer_completion()) have to wait for transfer submission
to fully complete. This eliminates any possible race conditions.
Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>
[hdegoede@redhat.com: Reworked libusb_submit_transfer changes so that in
case both flying_transfer_lock and itransfer->lock are taken
flying_transfers_lock is always taken first]
[hdegoede@redhat.com: Removed some unrelated changes (will be submitted
as separate patches)]
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
libusb/core.c | 4 +-
libusb/io.c | 144 ++++++++++++++++++++++++++-----------------------
libusb/libusbi.h | 43 +++++++--------
libusb/os/darwin_usb.c | 10 ++--
libusb/version_nano.h | 2 +-
5 files changed, 104 insertions(+), 99 deletions(-)
diff --git a/libusb/core.c b/libusb/core.c
index 2e3816c..d1e93c8 100644
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -1351,10 +1351,10 @@ static void do_close(struct libusb_context *ctx,
if (transfer->dev_handle != dev_handle)
continue;
- if (!(itransfer->flags & USBI_TRANSFER_DEVICE_DISAPPEARED)) {
+ if (!(itransfer->state_flags & USBI_TRANSFER_DEVICE_DISAPPEARED)) {
usbi_err(ctx, "Device handle closed while transfer was still being processed, but the device is still connected as far as we know");
- if (itransfer->flags & USBI_TRANSFER_CANCELLING)
+ if (itransfer->state_flags & USBI_TRANSFER_CANCELLING)
usbi_warn(ctx, "A cancellation for an in-flight transfer hasn't completed but closing the device handle");
else
usbi_err(ctx, "A cancellation hasn't even been scheduled on the transfer for which the device is closing");
diff --git a/libusb/io.c b/libusb/io.c
index 3bd1675..3757f44 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1269,7 +1269,6 @@ struct libusb_transfer * LIBUSB_CALL libusb_alloc_transfer(
itransfer->num_iso_packets = iso_packets;
usbi_mutex_init(&itransfer->lock);
- usbi_mutex_init(&itransfer->flags_lock);
transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
usbi_dbg("transfer %p", transfer);
return transfer;
@@ -1304,7 +1303,6 @@ void API_EXPORTED libusb_free_transfer(struct libusb_transfer *transfer)
itransfer = LIBUSB_TRANSFER_TO_USBI_TRANSFER(transfer);
usbi_mutex_destroy(&itransfer->lock);
- usbi_mutex_destroy(&itransfer->flags_lock);
free(itransfer);
}
@@ -1339,8 +1337,8 @@ static int arm_timerfd_for_next_timeout(struct libusb_context *ctx)
if (!timerisset(cur_tv))
goto disarm;
- /* act on first transfer that is not already cancelled */
- if (!(transfer->flags & USBI_TRANSFER_TIMEOUT_HANDLED)) {
+ /* act on first transfer that has not already been handled */
+ if (!(transfer->timeout_flags & USBI_TRANSFER_TIMEOUT_HANDLED)) {
int r;
const struct itimerspec it = { {0, 0},
{ cur_tv->tv_sec, cur_tv->tv_usec * 1000 } };
@@ -1374,8 +1372,6 @@ static int add_to_flying_list(struct usbi_transfer *transfer)
int r = 0;
int first = 1;
- usbi_mutex_lock(&ctx->flying_transfers_lock);
-
/* if we have no other flying transfers, start the list with this one */
if (list_empty(&ctx->flying_transfers)) {
list_add(&transfer->list, &ctx->flying_transfers);
@@ -1428,7 +1424,6 @@ out:
if (r)
list_del(&transfer->list);
- usbi_mutex_unlock(&ctx->flying_transfers_lock);
return r;
}
@@ -1471,62 +1466,79 @@ int API_EXPORTED libusb_submit_transfer(struct libusb_transfer *transfer)
{
struct usbi_transfer *itransfer =
LIBUSB_TRANSFER_TO_USBI_TRANSFER(transfer);
- int remove = 0;
+ struct libusb_context *ctx = TRANSFER_CTX(transfer);
int r;
usbi_dbg("transfer %p", transfer);
+
+ /*
+ * Important note on locking, this function takes / releases locks
+ * in the following order:
+ * take flying_transfers_lock
+ * take itransfer->lock
+ * clear transfer
+ * add to flying_transfers list
+ * release flying_transfers_lock
+ * submit transfer
+ * release itransfer->lock
+ * if submit failed:
+ * take flying_transfers_lock
+ * remove from flying_transfers list
+ * release flying_transfers_lock
+ *
+ * Note that it takes locks in the order a-b and then releases them
+ * in the same order a-b. This is somewhat unusual but not wrong,
+ * release order is not important as long as *all* locks are released
+ * before re-acquiring any locks.
+ *
+ * This means that the ordering of first releasing itransfer->lock
+ * and then re-acquiring the flying_transfers_list on error is
+ * important and must not be changed!
+ *
+ * This is done this way because when we take both locks we must always
+ * take flying_transfers_lock first to avoid ab-ba style deadlocks with
+ * the timeout handling and usbi_handle_disconnect paths.
+ *
+ * And we cannot release itransfer->lock before the submission is
+ * complete otherwise timeout handling for transfers with short
+ * timeouts may run before submission.
+ */
+ usbi_mutex_lock(&ctx->flying_transfers_lock);
usbi_mutex_lock(&itransfer->lock);
- usbi_mutex_lock(&itransfer->flags_lock);
- if (itransfer->flags & USBI_TRANSFER_IN_FLIGHT) {
- r = LIBUSB_ERROR_BUSY;
- goto out;
+ if (itransfer->state_flags & USBI_TRANSFER_IN_FLIGHT) {
+ usbi_mutex_unlock(&ctx->flying_transfers_lock);
+ usbi_mutex_unlock(&itransfer->lock);
+ return LIBUSB_ERROR_BUSY;
}
itransfer->transferred = 0;
- itransfer->flags = 0;
+ itransfer->state_flags = 0;
+ itransfer->timeout_flags = 0;
r = calculate_timeout(itransfer);
if (r < 0) {
- r = LIBUSB_ERROR_OTHER;
- goto out;
+ usbi_mutex_unlock(&ctx->flying_transfers_lock);
+ usbi_mutex_unlock(&itransfer->lock);
+ return LIBUSB_ERROR_OTHER;
}
- itransfer->flags |= USBI_TRANSFER_SUBMITTING;
- usbi_mutex_unlock(&itransfer->flags_lock);
r = add_to_flying_list(itransfer);
if (r) {
- usbi_mutex_lock(&itransfer->flags_lock);
- itransfer->flags = 0;
- goto out;
+ usbi_mutex_unlock(&ctx->flying_transfers_lock);
+ usbi_mutex_unlock(&itransfer->lock);
+ return r;
}
+ usbi_mutex_unlock(&ctx->flying_transfers_lock);
- /* keep a reference to this device */
- libusb_ref_device(transfer->dev_handle->dev);
r = usbi_backend->submit_transfer(itransfer);
-
- usbi_mutex_lock(&itransfer->flags_lock);
- itransfer->flags &= ~USBI_TRANSFER_SUBMITTING;
if (r == LIBUSB_SUCCESS) {
- /* check for two possible special conditions:
- * 1) device disconnect occurred immediately after submission
- * 2) transfer completed before we got here to update the flags
- */
- if (itransfer->flags & USBI_TRANSFER_DEVICE_DISAPPEARED) {
- usbi_backend->clear_transfer_priv(itransfer);
- remove = 1;
- r = LIBUSB_ERROR_NO_DEVICE;
- }
- else if (!(itransfer->flags & USBI_TRANSFER_COMPLETED)) {
- itransfer->flags |= USBI_TRANSFER_IN_FLIGHT;
- }
- } else {
- remove = 1;
- }
-out:
- usbi_mutex_unlock(&itransfer->flags_lock);
- if (remove) {
- libusb_unref_device(transfer->dev_handle->dev);
- remove_from_flying_list(itransfer);
+ itransfer->state_flags |= USBI_TRANSFER_IN_FLIGHT;
+ /* keep a reference to this device */
+ libusb_ref_device(transfer->dev_handle->dev);
}
usbi_mutex_unlock(&itransfer->lock);
+
+ if (r != LIBUSB_SUCCESS)
+ remove_from_flying_list(itransfer);
+
return r;
}
@@ -1552,9 +1564,8 @@ int API_EXPORTED libusb_cancel_transfer(struct libusb_transfer *transfer)
usbi_dbg("transfer %p", transfer );
usbi_mutex_lock(&itransfer->lock);
- usbi_mutex_lock(&itransfer->flags_lock);
- if (!(itransfer->flags & USBI_TRANSFER_IN_FLIGHT)
- || (itransfer->flags & USBI_TRANSFER_CANCELLING)) {
+ if (!(itransfer->state_flags & USBI_TRANSFER_IN_FLIGHT)
+ || (itransfer->state_flags & USBI_TRANSFER_CANCELLING)) {
r = LIBUSB_ERROR_NOT_FOUND;
goto out;
}
@@ -1568,13 +1579,12 @@ int API_EXPORTED libusb_cancel_transfer(struct libusb_transfer *transfer)
usbi_dbg("cancel transfer failed error %d", r);
if (r == LIBUSB_ERROR_NO_DEVICE)
- itransfer->flags |= USBI_TRANSFER_DEVICE_DISAPPEARED;
+ itransfer->state_flags |= USBI_TRANSFER_DEVICE_DISAPPEARED;
}
- itransfer->flags |= USBI_TRANSFER_CANCELLING;
+ itransfer->state_flags |= USBI_TRANSFER_CANCELLING;
out:
- usbi_mutex_unlock(&itransfer->flags_lock);
usbi_mutex_unlock(&itransfer->lock);
return r;
}
@@ -1637,10 +1647,9 @@ int usbi_handle_transfer_completion(struct usbi_transfer *itransfer,
if (r < 0)
usbi_err(ITRANSFER_CTX(itransfer), "failed to set timer for next timeout, errno=%d", errno);
- usbi_mutex_lock(&itransfer->flags_lock);
- itransfer->flags &= ~USBI_TRANSFER_IN_FLIGHT;
- itransfer->flags |= USBI_TRANSFER_COMPLETED;
- usbi_mutex_unlock(&itransfer->flags_lock);
+ usbi_mutex_lock(&itransfer->lock);
+ itransfer->state_flags &= ~USBI_TRANSFER_IN_FLIGHT;
+ usbi_mutex_unlock(&itransfer->lock);
if (status == LIBUSB_TRANSFER_COMPLETED
&& transfer->flags & LIBUSB_TRANSFER_SHORT_NOT_OK) {
@@ -1676,7 +1685,7 @@ int usbi_handle_transfer_completion(struct usbi_transfer *itransfer,
int usbi_handle_transfer_cancellation(struct usbi_transfer *transfer)
{
/* if the URB was cancelled due to timeout, report timeout to the user */
- if (transfer->flags & USBI_TRANSFER_TIMED_OUT) {
+ if (transfer->timeout_flags & USBI_TRANSFER_TIMED_OUT) {
usbi_dbg("detected timeout cancellation");
return usbi_handle_transfer_completion(transfer, LIBUSB_TRANSFER_TIMED_OUT);
}
@@ -1967,10 +1976,10 @@ static void handle_timeout(struct usbi_transfer *itransfer)
USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
int r;
- itransfer->flags |= USBI_TRANSFER_TIMEOUT_HANDLED;
+ itransfer->timeout_flags |= USBI_TRANSFER_TIMEOUT_HANDLED;
r = libusb_cancel_transfer(transfer);
if (r == 0)
- itransfer->flags |= USBI_TRANSFER_TIMED_OUT;
+ itransfer->timeout_flags |= USBI_TRANSFER_TIMED_OUT;
else
usbi_warn(TRANSFER_CTX(transfer),
"async cancel failed %d errno=%d", r, errno);
@@ -2003,7 +2012,7 @@ static int handle_timeouts_locked(struct libusb_context *ctx)
return 0;
/* ignore timeouts we've already handled */
- if (transfer->flags & (USBI_TRANSFER_TIMEOUT_HANDLED | USBI_TRANSFER_OS_HANDLES_TIMEOUT))
+ if (transfer->timeout_flags & (USBI_TRANSFER_TIMEOUT_HANDLED | USBI_TRANSFER_OS_HANDLES_TIMEOUT))
continue;
/* if transfer has non-expired timeout, nothing more to do */
@@ -2549,7 +2558,7 @@ int API_EXPORTED libusb_get_next_timeout(libusb_context *ctx,
/* find next transfer which hasn't already been processed as timed out */
list_for_each_entry(transfer, &ctx->flying_transfers, list, struct usbi_transfer) {
- if (transfer->flags & (USBI_TRANSFER_TIMEOUT_HANDLED | USBI_TRANSFER_OS_HANDLES_TIMEOUT))
+ if (transfer->timeout_flags & (USBI_TRANSFER_TIMEOUT_HANDLED | USBI_TRANSFER_OS_HANDLES_TIMEOUT))
continue;
/* if we've reached transfers of infinte timeout, we're done looking */
@@ -2766,9 +2775,10 @@ void usbi_handle_disconnect(struct libusb_device_handle *dev_handle)
* possible scenarios:
* 1. the transfer is currently in-flight, in which case we terminate the
* transfer here
- * 2. the transfer is not in-flight (or is but hasn't been marked as such),
- * in which case we record that the device disappeared and this will be
- * handled by libusb_submit_transfer()
+ * 2. the transfer has been added to the flying transfer list by
+ * libusb_submit_transfer, has failed to submit and
+ * libusb_submit_transfer is waiting for us to release the
+ * flying_transfers_lock to remove it, so we ignore it
*/
while (1) {
@@ -2776,12 +2786,10 @@ void usbi_handle_disconnect(struct libusb_device_handle *dev_handle)
usbi_mutex_lock(&HANDLE_CTX(dev_handle)->flying_transfers_lock);
list_for_each_entry(cur, &HANDLE_CTX(dev_handle)->flying_transfers, list, struct usbi_transfer)
if (USBI_TRANSFER_TO_LIBUSB_TRANSFER(cur)->dev_handle == dev_handle) {
- usbi_mutex_lock(&cur->flags_lock);
- if (cur->flags & USBI_TRANSFER_IN_FLIGHT)
+ usbi_mutex_lock(&cur->lock);
+ if (cur->state_flags & USBI_TRANSFER_IN_FLIGHT)
to_cancel = cur;
- else
- cur->flags |= USBI_TRANSFER_DEVICE_DISAPPEARED;
- usbi_mutex_unlock(&cur->flags_lock);
+ usbi_mutex_unlock(&cur->lock);
if (to_cancel)
break;
diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index ca54a46..cc0906c 100644
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -277,6 +277,8 @@ struct libusb_context {
* the list, URBs that will time out later are placed after, and urbs with
* infinite timeout are always placed at the very end. */
struct list_head flying_transfers;
+ /* Note paths taking both this and usbi_transfer->lock must always
+ * take this lock first */
usbi_mutex_t flying_transfers_lock;
/* user callbacks for pollfd changes */
@@ -440,7 +442,8 @@ struct usbi_transfer {
struct timeval timeout;
int transferred;
uint32_t stream_id;
- uint8_t flags;
+ uint8_t state_flags; /* Protected by usbi_transfer->lock */
+ uint8_t timeout_flags; /* Protected by the flying_stransfers_lock */
/* this lock is held during libusb_submit_transfer() and
* libusb_cancel_transfer() (allowing the OS backend to prevent duplicate
@@ -448,38 +451,32 @@ struct usbi_transfer {
* should also take this lock in the handle_events path, to prevent the user
* cancelling the transfer from another thread while you are processing
* its completion (presumably there would be races within your OS backend
- * if this were possible). */
+ * if this were possible).
+ * Note paths taking both this and the flying_transfers_lock must
+ * always take the flying_transfers_lock first */
usbi_mutex_t lock;
-
- /* this lock should be held whenever viewing or modifying flags
- * relating to the transfer state */
- usbi_mutex_t flags_lock;
};
-enum usbi_transfer_flags {
- /* The transfer has timed out */
- USBI_TRANSFER_TIMED_OUT = 1 << 0,
-
- /* Set by backend submit_transfer() if the OS handles timeout */
- USBI_TRANSFER_OS_HANDLES_TIMEOUT = 1 << 1,
+enum usbi_transfer_state_flags {
+ /* Transfer successfully submitted by backend */
+ USBI_TRANSFER_IN_FLIGHT = 1 << 0,
/* Cancellation was requested via libusb_cancel_transfer() */
- USBI_TRANSFER_CANCELLING = 1 << 2,
+ USBI_TRANSFER_CANCELLING = 1 << 1,
/* Operation on the transfer failed because the device disappeared */
- USBI_TRANSFER_DEVICE_DISAPPEARED = 1 << 3,
-
- /* Transfer is currently being submitted */
- USBI_TRANSFER_SUBMITTING = 1 << 4,
-
- /* Transfer successfully submitted by backend */
- USBI_TRANSFER_IN_FLIGHT = 1 << 5,
+ USBI_TRANSFER_DEVICE_DISAPPEARED = 1 << 2,
+};
- /* Completion handler has run */
- USBI_TRANSFER_COMPLETED = 1 << 6,
+enum usbi_transfer_timeout_flags {
+ /* Set by backend submit_transfer() if the OS handles timeout */
+ USBI_TRANSFER_OS_HANDLES_TIMEOUT = 1 << 0,
/* The transfer timeout has been handled */
- USBI_TRANSFER_TIMEOUT_HANDLED = 1 << 7,
+ USBI_TRANSFER_TIMEOUT_HANDLED = 1 << 1,
+
+ /* The transfer timeout was successfully processed */
+ USBI_TRANSFER_TIMED_OUT = 1 << 2,
};
#define USBI_TRANSFER_TO_LIBUSB_TRANSFER(transfer) \
diff --git a/libusb/os/darwin_usb.c b/libusb/os/darwin_usb.c
index 4e4ab04..7912282 100644
--- a/libusb/os/darwin_usb.c
+++ b/libusb/os/darwin_usb.c
@@ -1561,7 +1561,7 @@ static int submit_bulk_transfer(struct usbi_transfer *itransfer) {
ret = (*(cInterface->interface))->WritePipeAsync(cInterface->interface, pipeRef, transfer->buffer,
transfer->length, darwin_async_io_callback, itransfer);
} else {
- itransfer->flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT;
+ itransfer->timeout_flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT;
if (IS_XFERIN(transfer))
ret = (*(cInterface->interface))->ReadPipeAsyncTO(cInterface->interface, pipeRef, transfer->buffer,
@@ -1593,7 +1593,7 @@ static int submit_stream_transfer(struct usbi_transfer *itransfer) {
return LIBUSB_ERROR_NOT_FOUND;
}
- itransfer->flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT;
+ itransfer->timeout_flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT;
if (IS_XFERIN(transfer))
ret = (*(cInterface->interface))->ReadStreamsPipeAsyncTO(cInterface->interface, pipeRef, itransfer->stream_id,
@@ -1721,7 +1721,7 @@ static int submit_control_transfer(struct usbi_transfer *itransfer) {
tpriv->req.completionTimeout = transfer->timeout;
tpriv->req.noDataTimeout = transfer->timeout;
- itransfer->flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT;
+ itransfer->timeout_flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT;
/* all transfers in libusb-1.0 are async */
@@ -1870,7 +1870,7 @@ static void darwin_async_io_callback (void *refcon, IOReturn result, void *arg0)
}
static int darwin_transfer_status (struct usbi_transfer *itransfer, kern_return_t result) {
- if (itransfer->flags & USBI_TRANSFER_TIMED_OUT)
+ if (itransfer->timeout_flags & USBI_TRANSFER_TIMED_OUT)
result = kIOUSBTransactionTimeout;
switch (result) {
@@ -1887,7 +1887,7 @@ static int darwin_transfer_status (struct usbi_transfer *itransfer, kern_return_
return LIBUSB_TRANSFER_OVERFLOW;
case kIOUSBTransactionTimeout:
usbi_warn (ITRANSFER_CTX (itransfer), "transfer error: timed out");
- itransfer->flags |= USBI_TRANSFER_TIMED_OUT;
+ itransfer->timeout_flags |= USBI_TRANSFER_TIMED_OUT;
return LIBUSB_TRANSFER_TIMED_OUT;
default:
usbi_warn (ITRANSFER_CTX (itransfer), "transfer error: %s (value = 0x%08x)", darwin_error_str (result), result);
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index 3fdb625..de73e86 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11131
+#define LIBUSB_NANO 11132
--
2.7.4
From df1d10813702d572e55a27dbd9eea918dd02d24d Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Mon, 6 Jun 2016 16:53:53 +0200
Subject: [PATCH 06/11] core: Do not arm timer-fd for transfers where the os
handles timeout
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
libusb/io.c | 2 +-
libusb/version_nano.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libusb/io.c b/libusb/io.c
index 3757f44..13388cc 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1338,7 +1338,7 @@ static int arm_timerfd_for_next_timeout(struct libusb_context *ctx)
goto disarm;
/* act on first transfer that has not already been handled */
- if (!(transfer->timeout_flags & USBI_TRANSFER_TIMEOUT_HANDLED)) {
+ if (!(transfer->timeout_flags & (USBI_TRANSFER_TIMEOUT_HANDLED | USBI_TRANSFER_OS_HANDLES_TIMEOUT))) {
int r;
const struct itimerspec it = { {0, 0},
{ cur_tv->tv_sec, cur_tv->tv_usec * 1000 } };
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index de73e86..f39bf48 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11132
+#define LIBUSB_NANO 11133
--
2.7.4
From 1277fdd207fe041d0afc5e70b80aeb803f71f402 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Mon, 6 Jun 2016 16:54:42 +0200
Subject: [PATCH 07/11] core: Test for LIBUSB_SUCCESS instead of 0 in
handle_timeout()
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
libusb/io.c | 2 +-
libusb/version_nano.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libusb/io.c b/libusb/io.c
index 13388cc..aa6cfca 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1978,7 +1978,7 @@ static void handle_timeout(struct usbi_transfer *itransfer)
itransfer->timeout_flags |= USBI_TRANSFER_TIMEOUT_HANDLED;
r = libusb_cancel_transfer(transfer);
- if (r == 0)
+ if (r == LIBUSB_SUCCESS)
itransfer->timeout_flags |= USBI_TRANSFER_TIMED_OUT;
else
usbi_warn(TRANSFER_CTX(transfer),
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index f39bf48..9fb8855 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11133
+#define LIBUSB_NANO 11134
--
2.7.4
From ac98fdac04d409c63cae5416356414c14a37809a Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Mon, 6 Jun 2016 17:42:12 +0200
Subject: [PATCH 08/11] core: Fix do_close locking
Put the lock / unlock calls around the part of the code which actually
checks the flags which the lock protect.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
libusb/core.c | 4 ++--
libusb/version_nano.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/libusb/core.c b/libusb/core.c
index d1e93c8..99aab7b 100644
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -1351,6 +1351,7 @@ static void do_close(struct libusb_context *ctx,
if (transfer->dev_handle != dev_handle)
continue;
+ usbi_mutex_lock(&itransfer->lock);
if (!(itransfer->state_flags & USBI_TRANSFER_DEVICE_DISAPPEARED)) {
usbi_err(ctx, "Device handle closed while transfer was still being processed, but the device is still connected as far as we know");
@@ -1359,15 +1360,14 @@ static void do_close(struct libusb_context *ctx,
else
usbi_err(ctx, "A cancellation hasn't even been scheduled on the transfer for which the device is closing");
}
+ usbi_mutex_unlock(&itransfer->lock);
/* remove from the list of in-flight transfers and make sure
* we don't accidentally use the device handle in the future
* (or that such accesses will be easily caught and identified as a crash)
*/
- usbi_mutex_lock(&itransfer->lock);
list_del(&itransfer->list);
transfer->dev_handle = NULL;
- usbi_mutex_unlock(&itransfer->lock);
/* it is up to the user to free up the actual transfer struct. this is
* just making sure that we don't attempt to process the transfer after
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index 9fb8855..6995b33 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11134
+#define LIBUSB_NANO 11135
--
2.7.4
From 8380f1a8a41eab5afb9b5240b0b7db22dce28f8e Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Mon, 6 Jun 2016 17:43:23 +0200
Subject: [PATCH 09/11] core: Fix unlocked access to timeout_flags
There is a race between handle_timeout() and the completion functions.
When one thread is in handle_timeout() and another thread wakes
up from a poll(), there exists a window where the transfer has been
cancelled, but USBI_TRANSFER_TIMED_OUT is not yet set in timeout_flags.
Therefore, usbi_handle_transfer_completion() is sometimes called
with LIBUSB_TRANSFER_CANCELLED instead of the expected
LIBUSB_TRANSFER_TIMED_OUT.
timeout_flags is protected by the flying_transfers_lock, this commit
makes usbi_handle_transfer_cancellation() take that lock before
checking for USBI_TRANSFER_TIMED_OUT in timeout_flags, fixing this.
Reported-by: Joost Muller <joostmuller@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
libusb/io.c | 9 ++++++++-
libusb/version_nano.h | 2 +-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/libusb/io.c b/libusb/io.c
index aa6cfca..b3f7df0 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1684,8 +1684,15 @@ int usbi_handle_transfer_completion(struct usbi_transfer *itransfer,
* will attempt to take the lock. */
int usbi_handle_transfer_cancellation(struct usbi_transfer *transfer)
{
+ struct libusb_context *ctx = ITRANSFER_CTX(transfer);
+ uint8_t timed_out;
+
+ usbi_mutex_lock(&ctx->flying_transfers_lock);
+ timed_out = transfer->timeout_flags & USBI_TRANSFER_TIMED_OUT;
+ usbi_mutex_unlock(&ctx->flying_transfers_lock);
+
/* if the URB was cancelled due to timeout, report timeout to the user */
- if (transfer->timeout_flags & USBI_TRANSFER_TIMED_OUT) {
+ if (timed_out) {
usbi_dbg("detected timeout cancellation");
return usbi_handle_transfer_completion(transfer, LIBUSB_TRANSFER_TIMED_OUT);
}
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index 6995b33..bb7624f 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11135
+#define LIBUSB_NANO 11137
--
2.7.4
From 23a8fb757f7eeca4850116b88aac2ea8d34b935f Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Wed, 8 Jun 2016 10:34:05 +0200
Subject: [PATCH 10/11] core: Move calculate_timeout call to
add_to_flying_transfers
This cleans-up libusb_submit_transfer a bit by avoiding an error
exit path with unlock calls.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
libusb/io.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/libusb/io.c b/libusb/io.c
index b3f7df0..8363628 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1369,9 +1369,13 @@ static int add_to_flying_list(struct usbi_transfer *transfer)
struct usbi_transfer *cur;
struct timeval *timeout = &transfer->timeout;
struct libusb_context *ctx = ITRANSFER_CTX(transfer);
- int r = 0;
+ int r;
int first = 1;
+ r = calculate_timeout(transfer);
+ if (r)
+ return r;
+
/* if we have no other flying transfers, start the list with this one */
if (list_empty(&ctx->flying_transfers)) {
list_add(&transfer->list, &ctx->flying_transfers);
@@ -1513,13 +1517,6 @@ int API_EXPORTED libusb_submit_transfer(struct libusb_transfer *transfer)
itransfer->transferred = 0;
itransfer->state_flags = 0;
itransfer->timeout_flags = 0;
- r = calculate_timeout(itransfer);
- if (r < 0) {
- usbi_mutex_unlock(&ctx->flying_transfers_lock);
- usbi_mutex_unlock(&itransfer->lock);
- return LIBUSB_ERROR_OTHER;
- }
-
r = add_to_flying_list(itransfer);
if (r) {
usbi_mutex_unlock(&ctx->flying_transfers_lock);
--
2.7.4
From de378ea51c22eb7529812a69ab74a3b3eef33423 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Tue, 7 Jun 2016 12:12:38 +0200
Subject: [PATCH 11/11] linux_usbfs: Deal with receiving POLLERR before all
transfers have completed
The linux kernel will set its internal device state to USB_STATE_NOTATTACHED
as soon as it detects the disconnect, and then start a worker thread to
deal with the actual disconnection, kill outstanding urbs, etc.
The usbfs poll implementation will return POLL_ERR as soon as
ps->dev->state == USB_STATE_NOTATTACHED. The kernel will not wakeup
the poll until it is done with processing the disconnection. But if
we happen to call poll() between the state change and the disconnection
being fully processed, we may not be able to reap all outstanding transfers,
even on kernels with the USBFS_CAP_REAP_AFTER_DISCONNECT capability.
This commit deals with this by trying to reap as many transfers as possible
on disconnect on USBFS_CAP_REAP_AFTER_DISCONNECT capable kernels and then
calling usbi_handle_disconnect(handle) to deal with any remaining ones.
On USBFS_CAP_REAP_AFTER_DISCONNECT capable kernels this will be a no-op
unless we hit the race.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
libusb/os/linux_usbfs.c | 10 +++++++---
libusb/version_nano.h | 2 +-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
index 0689894..9cbeb80 100644
--- a/libusb/os/linux_usbfs.c
+++ b/libusb/os/linux_usbfs.c
@@ -2638,10 +2638,14 @@ static int op_handle_events(struct libusb_context *ctx,
handle->dev->device_address);
usbi_mutex_static_unlock(&linux_hotplug_lock);
- if (!(hpriv->caps & USBFS_CAP_REAP_AFTER_DISCONNECT)) {
- usbi_handle_disconnect(handle);
- continue;
+ if (hpriv->caps & USBFS_CAP_REAP_AFTER_DISCONNECT) {
+ do {
+ r = reap_for_handle(handle);
+ } while (r == 0);
}
+
+ usbi_handle_disconnect(handle);
+ continue;
}
do {
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index bb7624f..b394a2e 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11137
+#define LIBUSB_NANO 11138
--
2.7.4