|
|
38df8a |
From 65115ea3b437612c5c75408d9a4028cdb070e406 Mon Sep 17 00:00:00 2001
|
|
|
38df8a |
From: Chris Dickens <christopher.a.dickens@gmail.com>
|
|
|
38df8a |
Date: Mon, 26 Oct 2015 14:18:33 +0100
|
|
|
38df8a |
Subject: [PATCH 3/3] core: Refactor code related to transfer flags and timeout
|
|
|
38df8a |
handling
|
|
|
38df8a |
|
|
|
38df8a |
Commit a886bb02 sped up the library a bit by removing the serialization
|
|
|
38df8a |
of transfer submission with respect to the flying_transfers list, but
|
|
|
38df8a |
it introduced two separate issues.
|
|
|
38df8a |
|
|
|
38df8a |
1) A deadlock scenario is possible given the following sequence:
|
|
|
38df8a |
|
|
|
38df8a |
- Thread A submits transfer with very short timeout (say 1ms)
|
|
|
38df8a |
-> takes transfer->lock
|
|
|
38df8a |
-> adds transfer to flying_transfers list and arms timerfd
|
|
|
38df8a |
-> actually calls backend to submit transfer, but it fails
|
|
|
38df8a |
<context switch>
|
|
|
38df8a |
- Thread B is doing event handling and sees the timerfd trigger
|
|
|
38df8a |
-> takes ctx->flying_transfers_lock
|
|
|
38df8a |
-> finds the transfer above on the list
|
|
|
38df8a |
-> calls libusb_cancel_transfer() for this transfer
|
|
|
38df8a |
|
|
|
38df8a |
<context switch>
|
|
|
38df8a |
- Thread A sees the transfer failed to submit
|
|
|
38df8a |
-> removes transfer from flying_transfers list
|
|
|
38df8a |
--> takes ctx->flying_transfers_lock (still holding transfer->lock)
|
|
|
38df8a |
** DEADLOCK **
|
|
|
38df8a |
|
|
|
38df8a |
2) The transfer state flags (e.g. submitting, in-flight) were protected
|
|
|
38df8a |
by transfer->flags_lock, but the timeout-related flags were OR'ed in
|
|
|
38df8a |
during timeout handling operations outside of the lock. This leads to
|
|
|
38df8a |
the possibility that transfer state might get overwritten.
|
|
|
38df8a |
|
|
|
38df8a |
This change corrects these issues and simplifies the transfer submission
|
|
|
38df8a |
code a bit by separating the state and timeout flags into their own flag
|
|
|
38df8a |
variables. The state flags are protected by the transfer lock. The timeout
|
|
|
38df8a |
flags are protected by the flying_transfers_lock.
|
|
|
38df8a |
|
|
|
38df8a |
The transfer submission code sheds some weight because it no longer needs
|
|
|
38df8a |
to worry about the timing of events that modify the transfer state flags.
|
|
|
38df8a |
These flags are always viewed and modified under the protection of the
|
|
|
38df8a |
transfer lock. Since libusb_submit_transfer() holds the transfer lock for
|
|
|
38df8a |
the entire duration of the operation, the other code paths that would
|
|
|
38df8a |
possibly touch the transfer (e.g. usbi_handle_disconnect() and
|
|
|
38df8a |
usbi_handle_transfer_completion()) have to wait for transfer submission
|
|
|
38df8a |
to fully complete. This eliminates any possible race conditions.
|
|
|
38df8a |
|
|
|
38df8a |
Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>
|
|
|
38df8a |
[hdegoede@redhat.com: Reworked libusb_submit_transfer changes so that in
|
|
|
38df8a |
case both flying_transfer_lock and itransfer->lock are taken
|
|
|
38df8a |
flying_transfers_lock is always taken first]
|
|
|
38df8a |
[hdegoede@redhat.com: Removed some unrelated changes (will be submitted
|
|
|
38df8a |
as separate patches)]
|
|
|
38df8a |
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
|
|
|
38df8a |
|
|
|
38df8a |
libusb/core.c | 4 +-
|
|
|
38df8a |
libusb/io.c | 144 ++++++++++++++++++++++++++
|
|
|
38df8a |
libusb/libusbi.h | 43 +++++++
|
|
|
38df8a |
libusb/os/darwin_usb.c | 10 ++
|
|
|
38df8a |
libusb/version_nano.h | 2 +-
|
|
|
38df8a |
5 files changed, 104 insertions(+), 99 deletions(-)
|
|
|
38df8a |
|
|
|
38df8a |
diff
|
|
|
38df8a |
index 5884dcf..837e8f2 100644
|
|
|
38df8a |
|
|
|
38df8a |
|
|
|
38df8a |
@@ -1331,10 +1331,10 @@ static void do_close(struct libusb_context *ctx,
|
|
|
38df8a |
if (transfer->dev_handle != dev_handle)
|
|
|
38df8a |
continue;
|
|
|
38df8a |
|
|
|
38df8a |
- if (!(itransfer->flags & USBI_TRANSFER_DEVICE_DISAPPEARED)) {
|
|
|
38df8a |
+ if (!(itransfer->state_flags & USBI_TRANSFER_DEVICE_DISAPPEARED)) {
|
|
|
38df8a |
usbi_err(ctx, "Device handle closed while transfer was still being processed, but the device is still connected as far as we know");
|
|
|
38df8a |
|
|
|
38df8a |
- if (itransfer->flags & USBI_TRANSFER_CANCELLING)
|
|
|
38df8a |
+ if (itransfer->state_flags & USBI_TRANSFER_CANCELLING)
|
|
|
38df8a |
usbi_warn(ctx, "A cancellation for an in-flight transfer hasn't completed but closing the device handle");
|
|
|
38df8a |
else
|
|
|
38df8a |
usbi_err(ctx, "A cancellation hasn't even been scheduled on the transfer for which the device is closing");
|
|
|
38df8a |
diff
|
|
|
38df8a |
index bc85438..c216814 100644
|
|
|
38df8a |
|
|
|
38df8a |
+++ b/libusb/io.c
|
|
|
38df8a |
@@ -1260,7 +1260,6 @@ struct libusb_transfer * LIBUSB_CALL libusb_alloc_transfer(
|
|
|
38df8a |
|
|
|
38df8a |
itransfer->num_iso_packets = iso_packets;
|
|
|
38df8a |
usbi_mutex_init(&itransfer->lock, NULL);
|
|
|
38df8a |
- usbi_mutex_init(&itransfer->flags_lock, NULL);
|
|
|
38df8a |
transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
|
|
|
38df8a |
usbi_dbg("transfer %p", transfer);
|
|
|
38df8a |
return transfer;
|
|
|
38df8a |
@@ -1295,7 +1294,6 @@ void API_EXPORTED libusb_free_transfer(struct libusb_transfer *transfer)
|
|
|
38df8a |
|
|
|
38df8a |
itransfer = LIBUSB_TRANSFER_TO_USBI_TRANSFER(transfer);
|
|
|
38df8a |
usbi_mutex_destroy(&itransfer->lock);
|
|
|
38df8a |
- usbi_mutex_destroy(&itransfer->flags_lock);
|
|
|
38df8a |
free(itransfer);
|
|
|
38df8a |
}
|
|
|
38df8a |
|
|
|
38df8a |
@@ -1330,8 +1328,8 @@ static int arm_timerfd_for_next_timeout(struct libusb_context *ctx)
|
|
|
38df8a |
if (!timerisset(cur_tv))
|
|
|
38df8a |
goto disarm;
|
|
|
38df8a |
|
|
|
38df8a |
- /* act on first transfer that is not already cancelled */
|
|
|
38df8a |
- if (!(transfer->flags & USBI_TRANSFER_TIMEOUT_HANDLED)) {
|
|
|
38df8a |
+ /* act on first transfer that has not already been handled */
|
|
|
38df8a |
+ if (!(transfer->timeout_flags & USBI_TRANSFER_TIMEOUT_HANDLED)) {
|
|
|
38df8a |
int r;
|
|
|
38df8a |
const struct itimerspec it = { {0, 0},
|
|
|
38df8a |
{ cur_tv->tv_sec, cur_tv->tv_usec * 1000 } };
|
|
|
38df8a |
@@ -1365,8 +1363,6 @@ static int add_to_flying_list(struct usbi_transfer *transfer)
|
|
|
38df8a |
int r = 0;
|
|
|
38df8a |
int first = 1;
|
|
|
38df8a |
|
|
|
38df8a |
- usbi_mutex_lock(&ctx->flying_transfers_lock);
|
|
|
38df8a |
-
|
|
|
38df8a |
|
|
|
38df8a |
if (list_empty(&ctx->flying_transfers)) {
|
|
|
38df8a |
list_add(&transfer->list, &ctx->flying_transfers);
|
|
|
38df8a |
@@ -1419,7 +1415,6 @@ out:
|
|
|
38df8a |
if (r)
|
|
|
38df8a |
list_del(&transfer->list);
|
|
|
38df8a |
|
|
|
38df8a |
- usbi_mutex_unlock(&ctx->flying_transfers_lock);
|
|
|
38df8a |
return r;
|
|
|
38df8a |
}
|
|
|
38df8a |
|
|
|
38df8a |
@@ -1460,62 +1455,79 @@ int API_EXPORTED libusb_submit_transfer(struct libusb_transfer *transfer)
|
|
|
38df8a |
{
|
|
|
38df8a |
struct usbi_transfer *itransfer =
|
|
|
38df8a |
LIBUSB_TRANSFER_TO_USBI_TRANSFER(transfer);
|
|
|
38df8a |
- int remove = 0;
|
|
|
38df8a |
+ struct libusb_context *ctx = TRANSFER_CTX(transfer);
|
|
|
38df8a |
int r;
|
|
|
38df8a |
|
|
|
38df8a |
usbi_dbg("transfer %p", transfer);
|
|
|
38df8a |
+
|
|
|
38df8a |
+ /*
|
|
|
38df8a |
+ * Important note on locking, this function takes / releases locks
|
|
|
38df8a |
+ * in the following order:
|
|
|
38df8a |
+ * take flying_transfers_lock
|
|
|
38df8a |
+ * take itransfer->lock
|
|
|
38df8a |
+ * clear transfer
|
|
|
38df8a |
+ * add to flying_transfers list
|
|
|
38df8a |
+ * release flying_transfers_lock
|
|
|
38df8a |
+ * submit transfer
|
|
|
38df8a |
+ * release itransfer->lock
|
|
|
38df8a |
+ * if submit failed:
|
|
|
38df8a |
+ * take flying_transfers_lock
|
|
|
38df8a |
+ * remove from flying_transfers list
|
|
|
38df8a |
+ * release flying_transfers_lock
|
|
|
38df8a |
+ *
|
|
|
38df8a |
+ * Note that it takes locks in the order a-b and then releases them
|
|
|
38df8a |
+ * in the same order a-b. This is somewhat unusual but not wrong,
|
|
|
38df8a |
+ * release order is not important as long as *all* locks are released
|
|
|
38df8a |
+ * before re-acquiring any locks.
|
|
|
38df8a |
+ *
|
|
|
38df8a |
+ * This means that the ordering of first releasing itransfer->lock
|
|
|
38df8a |
+ * and then re-acquiring the flying_transfers_list on error is
|
|
|
38df8a |
+ * important and must not be changed!
|
|
|
38df8a |
+ *
|
|
|
38df8a |
+ * This is done this way because when we take both locks we must always
|
|
|
38df8a |
+ * take flying_transfers_lock first to avoid ab-ba style deadlocks with
|
|
|
38df8a |
+ * the timeout handling and usbi_handle_disconnect paths.
|
|
|
38df8a |
+ *
|
|
|
38df8a |
+ * And we cannot release itransfer->lock before the submission is
|
|
|
38df8a |
+ * complete otherwise timeout handling for transfers with short
|
|
|
38df8a |
+ * timeouts may run before submission.
|
|
|
38df8a |
+ */
|
|
|
38df8a |
+ usbi_mutex_lock(&ctx->flying_transfers_lock);
|
|
|
38df8a |
usbi_mutex_lock(&itransfer->lock);
|
|
|
38df8a |
- usbi_mutex_lock(&itransfer->flags_lock);
|
|
|
38df8a |
- if (itransfer->flags & USBI_TRANSFER_IN_FLIGHT) {
|
|
|
38df8a |
- r = LIBUSB_ERROR_BUSY;
|
|
|
38df8a |
- goto out;
|
|
|
38df8a |
+ if (itransfer->state_flags & USBI_TRANSFER_IN_FLIGHT) {
|
|
|
38df8a |
+ usbi_mutex_unlock(&ctx->flying_transfers_lock);
|
|
|
38df8a |
+ usbi_mutex_unlock(&itransfer->lock);
|
|
|
38df8a |
+ return LIBUSB_ERROR_BUSY;
|
|
|
38df8a |
}
|
|
|
38df8a |
itransfer->transferred = 0;
|
|
|
38df8a |
- itransfer->flags = 0;
|
|
|
38df8a |
+ itransfer->state_flags = 0;
|
|
|
38df8a |
+ itransfer->timeout_flags = 0;
|
|
|
38df8a |
r = calculate_timeout(itransfer);
|
|
|
38df8a |
if (r < 0) {
|
|
|
38df8a |
- r = LIBUSB_ERROR_OTHER;
|
|
|
38df8a |
- goto out;
|
|
|
38df8a |
+ usbi_mutex_unlock(&ctx->flying_transfers_lock);
|
|
|
38df8a |
+ usbi_mutex_unlock(&itransfer->lock);
|
|
|
38df8a |
+ return LIBUSB_ERROR_OTHER;
|
|
|
38df8a |
}
|
|
|
38df8a |
- itransfer->flags |= USBI_TRANSFER_SUBMITTING;
|
|
|
38df8a |
- usbi_mutex_unlock(&itransfer->flags_lock);
|
|
|
38df8a |
|
|
|
38df8a |
r = add_to_flying_list(itransfer);
|
|
|
38df8a |
if (r) {
|
|
|
38df8a |
- usbi_mutex_lock(&itransfer->flags_lock);
|
|
|
38df8a |
- itransfer->flags = 0;
|
|
|
38df8a |
- goto out;
|
|
|
38df8a |
+ usbi_mutex_unlock(&ctx->flying_transfers_lock);
|
|
|
38df8a |
+ usbi_mutex_unlock(&itransfer->lock);
|
|
|
38df8a |
+ return r;
|
|
|
38df8a |
}
|
|
|
38df8a |
+ usbi_mutex_unlock(&ctx->flying_transfers_lock);
|
|
|
38df8a |
|
|
|
38df8a |
-
|
|
|
38df8a |
- libusb_ref_device(transfer->dev_handle->dev);
|
|
|
38df8a |
r = usbi_backend->submit_transfer(itransfer);
|
|
|
38df8a |
-
|
|
|
38df8a |
- usbi_mutex_lock(&itransfer->flags_lock);
|
|
|
38df8a |
- itransfer->flags &= ~USBI_TRANSFER_SUBMITTING;
|
|
|
38df8a |
if (r == LIBUSB_SUCCESS) {
|
|
|
38df8a |
- /* check for two possible special conditions:
|
|
|
38df8a |
- * 1) device disconnect occurred immediately after submission
|
|
|
38df8a |
- * 2) transfer completed before we got here to update the flags
|
|
|
38df8a |
- */
|
|
|
38df8a |
- if (itransfer->flags & USBI_TRANSFER_DEVICE_DISAPPEARED) {
|
|
|
38df8a |
- usbi_backend->clear_transfer_priv(itransfer);
|
|
|
38df8a |
- remove = 1;
|
|
|
38df8a |
- r = LIBUSB_ERROR_NO_DEVICE;
|
|
|
38df8a |
- }
|
|
|
38df8a |
- else if (!(itransfer->flags & USBI_TRANSFER_COMPLETED)) {
|
|
|
38df8a |
- itransfer->flags |= USBI_TRANSFER_IN_FLIGHT;
|
|
|
38df8a |
- }
|
|
|
38df8a |
- } else {
|
|
|
38df8a |
- remove = 1;
|
|
|
38df8a |
- }
|
|
|
38df8a |
-out:
|
|
|
38df8a |
- usbi_mutex_unlock(&itransfer->flags_lock);
|
|
|
38df8a |
- if (remove) {
|
|
|
38df8a |
- libusb_unref_device(transfer->dev_handle->dev);
|
|
|
38df8a |
- remove_from_flying_list(itransfer);
|
|
|
38df8a |
+ itransfer->state_flags |= USBI_TRANSFER_IN_FLIGHT;
|
|
|
38df8a |
+
|
|
|
38df8a |
+ libusb_ref_device(transfer->dev_handle->dev);
|
|
|
38df8a |
}
|
|
|
38df8a |
usbi_mutex_unlock(&itransfer->lock);
|
|
|
38df8a |
+
|
|
|
38df8a |
+ if (r != LIBUSB_SUCCESS)
|
|
|
38df8a |
+ remove_from_flying_list(itransfer);
|
|
|
38df8a |
+
|
|
|
38df8a |
return r;
|
|
|
38df8a |
}
|
|
|
38df8a |
|
|
|
38df8a |
@@ -1541,9 +1553,8 @@ int API_EXPORTED libusb_cancel_transfer(struct libusb_transfer *transfer)
|
|
|
38df8a |
|
|
|
38df8a |
usbi_dbg("transfer %p", transfer );
|
|
|
38df8a |
usbi_mutex_lock(&itransfer->lock);
|
|
|
38df8a |
- usbi_mutex_lock(&itransfer->flags_lock);
|
|
|
38df8a |
- if (!(itransfer->flags & USBI_TRANSFER_IN_FLIGHT)
|
|
|
38df8a |
- || (itransfer->flags & USBI_TRANSFER_CANCELLING)) {
|
|
|
38df8a |
+ if (!(itransfer->state_flags & USBI_TRANSFER_IN_FLIGHT)
|
|
|
38df8a |
+ || (itransfer->state_flags & USBI_TRANSFER_CANCELLING)) {
|
|
|
38df8a |
r = LIBUSB_ERROR_NOT_FOUND;
|
|
|
38df8a |
goto out;
|
|
|
38df8a |
}
|
|
|
38df8a |
@@ -1557,13 +1568,12 @@ int API_EXPORTED libusb_cancel_transfer(struct libusb_transfer *transfer)
|
|
|
38df8a |
usbi_dbg("cancel transfer failed error %d", r);
|
|
|
38df8a |
|
|
|
38df8a |
if (r == LIBUSB_ERROR_NO_DEVICE)
|
|
|
38df8a |
- itransfer->flags |= USBI_TRANSFER_DEVICE_DISAPPEARED;
|
|
|
38df8a |
+ itransfer->state_flags |= USBI_TRANSFER_DEVICE_DISAPPEARED;
|
|
|
38df8a |
}
|
|
|
38df8a |
|
|
|
38df8a |
- itransfer->flags |= USBI_TRANSFER_CANCELLING;
|
|
|
38df8a |
+ itransfer->state_flags |= USBI_TRANSFER_CANCELLING;
|
|
|
38df8a |
|
|
|
38df8a |
out:
|
|
|
38df8a |
- usbi_mutex_unlock(&itransfer->flags_lock);
|
|
|
38df8a |
usbi_mutex_unlock(&itransfer->lock);
|
|
|
38df8a |
return r;
|
|
|
38df8a |
}
|
|
|
38df8a |
@@ -1626,10 +1636,9 @@ int usbi_handle_transfer_completion(struct usbi_transfer *itransfer,
|
|
|
38df8a |
if (r < 0)
|
|
|
38df8a |
usbi_err(ITRANSFER_CTX(itransfer), "failed to set timer for next timeout, errno=%d", errno);
|
|
|
38df8a |
|
|
|
38df8a |
- usbi_mutex_lock(&itransfer->flags_lock);
|
|
|
38df8a |
- itransfer->flags &= ~USBI_TRANSFER_IN_FLIGHT;
|
|
|
38df8a |
- itransfer->flags |= USBI_TRANSFER_COMPLETED;
|
|
|
38df8a |
- usbi_mutex_unlock(&itransfer->flags_lock);
|
|
|
38df8a |
+ usbi_mutex_lock(&itransfer->lock);
|
|
|
38df8a |
+ itransfer->state_flags &= ~USBI_TRANSFER_IN_FLIGHT;
|
|
|
38df8a |
+ usbi_mutex_unlock(&itransfer->lock);
|
|
|
38df8a |
|
|
|
38df8a |
if (status == LIBUSB_TRANSFER_COMPLETED
|
|
|
38df8a |
&& transfer->flags & LIBUSB_TRANSFER_SHORT_NOT_OK) {
|
|
|
38df8a |
@@ -1665,7 +1674,7 @@ int usbi_handle_transfer_completion(struct usbi_transfer *itransfer,
|
|
|
38df8a |
int usbi_handle_transfer_cancellation(struct usbi_transfer *transfer)
|
|
|
38df8a |
{
|
|
|
38df8a |
|
|
|
38df8a |
- if (transfer->flags & USBI_TRANSFER_TIMED_OUT) {
|
|
|
38df8a |
+ if (transfer->timeout_flags & USBI_TRANSFER_TIMED_OUT) {
|
|
|
38df8a |
usbi_dbg("detected timeout cancellation");
|
|
|
38df8a |
return usbi_handle_transfer_completion(transfer, LIBUSB_TRANSFER_TIMED_OUT);
|
|
|
38df8a |
}
|
|
|
38df8a |
@@ -1966,10 +1975,10 @@ static void handle_timeout(struct usbi_transfer *itransfer)
|
|
|
38df8a |
USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
|
|
|
38df8a |
int r;
|
|
|
38df8a |
|
|
|
38df8a |
- itransfer->flags |= USBI_TRANSFER_TIMEOUT_HANDLED;
|
|
|
38df8a |
+ itransfer->timeout_flags |= USBI_TRANSFER_TIMEOUT_HANDLED;
|
|
|
38df8a |
r = libusb_cancel_transfer(transfer);
|
|
|
38df8a |
if (r == 0)
|
|
|
38df8a |
- itransfer->flags |= USBI_TRANSFER_TIMED_OUT;
|
|
|
38df8a |
+ itransfer->timeout_flags |= USBI_TRANSFER_TIMED_OUT;
|
|
|
38df8a |
else
|
|
|
38df8a |
usbi_warn(TRANSFER_CTX(transfer),
|
|
|
38df8a |
"async cancel failed %d errno=%d", r, errno);
|
|
|
38df8a |
@@ -2002,7 +2011,7 @@ static int handle_timeouts_locked(struct libusb_context *ctx)
|
|
|
38df8a |
return 0;
|
|
|
38df8a |
|
|
|
38df8a |
|
|
|
38df8a |
- if (transfer->flags & (USBI_TRANSFER_TIMEOUT_HANDLED | USBI_TRANSFER_OS_HANDLES_TIMEOUT))
|
|
|
38df8a |
+ if (transfer->timeout_flags & (USBI_TRANSFER_TIMEOUT_HANDLED | USBI_TRANSFER_OS_HANDLES_TIMEOUT))
|
|
|
38df8a |
continue;
|
|
|
38df8a |
|
|
|
38df8a |
/* if transfer has non-expired timeout, nothing more to do */
|
|
|
38df8a |
@@ -2534,7 +2543,7 @@ int API_EXPORTED libusb_get_next_timeout(libusb_context *ctx,
|
|
|
38df8a |
|
|
|
38df8a |
/* find next transfer which hasn't already been processed as timed out */
|
|
|
38df8a |
list_for_each_entry(transfer, &ctx->flying_transfers, list, struct usbi_transfer) {
|
|
|
38df8a |
- if (transfer->flags & (USBI_TRANSFER_TIMEOUT_HANDLED | USBI_TRANSFER_OS_HANDLES_TIMEOUT))
|
|
|
38df8a |
+ if (transfer->timeout_flags & (USBI_TRANSFER_TIMEOUT_HANDLED | USBI_TRANSFER_OS_HANDLES_TIMEOUT))
|
|
|
38df8a |
continue;
|
|
|
38df8a |
|
|
|
38df8a |
/* if we've reached transfers of infinte timeout, we're done looking */
|
|
|
38df8a |
@@ -2751,9 +2760,10 @@ void usbi_handle_disconnect(struct libusb_device_handle *handle)
|
|
|
38df8a |
* possible scenarios:
|
|
|
38df8a |
* 1. the transfer is currently in-flight, in which case we terminate the
|
|
|
38df8a |
* transfer here
|
|
|
38df8a |
- * 2. the transfer is not in-flight (or is but hasn't been marked as such),
|
|
|
38df8a |
- * in which case we record that the device disappeared and this will be
|
|
|
38df8a |
- * handled by libusb_submit_transfer()
|
|
|
38df8a |
+ * 2. the transfer has been added to the flying transfer list by
|
|
|
38df8a |
+ * libusb_submit_transfer, has failed to submit and
|
|
|
38df8a |
+ * libusb_submit_transfer is waiting for us to release the
|
|
|
38df8a |
+ * flying_transfers_lock to remove it, so we ignore it
|
|
|
38df8a |
*/
|
|
|
38df8a |
|
|
|
38df8a |
while (1) {
|
|
|
38df8a |
@@ -2761,12 +2771,10 @@ void usbi_handle_disconnect(struct libusb_device_handle *handle)
|
|
|
38df8a |
usbi_mutex_lock(&HANDLE_CTX(handle)->flying_transfers_lock);
|
|
|
38df8a |
list_for_each_entry(cur, &HANDLE_CTX(handle)->flying_transfers, list, struct usbi_transfer)
|
|
|
38df8a |
if (USBI_TRANSFER_TO_LIBUSB_TRANSFER(cur)->dev_handle == handle) {
|
|
|
38df8a |
- usbi_mutex_lock(&cur->flags_lock);
|
|
|
38df8a |
- if (cur->flags & USBI_TRANSFER_IN_FLIGHT)
|
|
|
38df8a |
+ usbi_mutex_lock(&cur->lock);
|
|
|
38df8a |
+ if (cur->state_flags & USBI_TRANSFER_IN_FLIGHT)
|
|
|
38df8a |
to_cancel = cur;
|
|
|
38df8a |
- else
|
|
|
38df8a |
- cur->flags |= USBI_TRANSFER_DEVICE_DISAPPEARED;
|
|
|
38df8a |
- usbi_mutex_unlock(&cur->flags_lock);
|
|
|
38df8a |
+ usbi_mutex_unlock(&cur->lock);
|
|
|
38df8a |
|
|
|
38df8a |
if (to_cancel)
|
|
|
38df8a |
break;
|
|
|
38df8a |
diff
|
|
|
38df8a |
index a39cbe1..95badd3 100644
|
|
|
38df8a |
|
|
|
38df8a |
|
|
|
38df8a |
@@ -266,6 +266,8 @@ struct libusb_context {
|
|
|
38df8a |
* the list, URBs that will time out later are placed after, and urbs with
|
|
|
38df8a |
* infinite timeout are always placed at the very end. */
|
|
|
38df8a |
struct list_head flying_transfers;
|
|
|
38df8a |
+ /* Note paths taking both this and usbi_transfer->lock must always
|
|
|
38df8a |
+ * take this lock first */
|
|
|
38df8a |
usbi_mutex_t flying_transfers_lock;
|
|
|
38df8a |
|
|
|
38df8a |
/* user callbacks for pollfd changes */
|
|
|
38df8a |
@@ -407,7 +409,8 @@ struct usbi_transfer {
|
|
|
38df8a |
struct timeval timeout;
|
|
|
38df8a |
int transferred;
|
|
|
38df8a |
uint32_t stream_id;
|
|
|
38df8a |
- uint8_t flags;
|
|
|
38df8a |
+ uint8_t state_flags; /* Protected by usbi_transfer->lock */
|
|
|
38df8a |
+ uint8_t timeout_flags;
|
|
|
38df8a |
|
|
|
38df8a |
/* this lock is held during libusb_submit_transfer() and
|
|
|
38df8a |
* libusb_cancel_transfer() (allowing the OS backend to prevent duplicate
|
|
|
38df8a |
@@ -415,38 +418,32 @@ struct usbi_transfer {
|
|
|
38df8a |
* should also take this lock in the handle_events path, to prevent the user
|
|
|
38df8a |
* cancelling the transfer from another thread while you are processing
|
|
|
38df8a |
* its completion (presumably there would be races within your OS backend
|
|
|
38df8a |
- * if this were possible). */
|
|
|
38df8a |
+ * if this were possible).
|
|
|
38df8a |
+ * Note paths taking both this and the flying_transfers_lock must
|
|
|
38df8a |
+ * always take the flying_transfers_lock first */
|
|
|
38df8a |
usbi_mutex_t lock;
|
|
|
38df8a |
-
|
|
|
38df8a |
-
|
|
|
38df8a |
- * relating to the transfer state */
|
|
|
38df8a |
- usbi_mutex_t flags_lock;
|
|
|
38df8a |
};
|
|
|
38df8a |
|
|
|
38df8a |
-enum usbi_transfer_flags {
|
|
|
38df8a |
- /* The transfer has timed out */
|
|
|
38df8a |
- USBI_TRANSFER_TIMED_OUT = 1 << 0,
|
|
|
38df8a |
-
|
|
|
38df8a |
-
|
|
|
38df8a |
- USBI_TRANSFER_OS_HANDLES_TIMEOUT = 1 << 1,
|
|
|
38df8a |
+enum usbi_transfer_state_flags {
|
|
|
38df8a |
+ /* Transfer successfully submitted by backend */
|
|
|
38df8a |
+ USBI_TRANSFER_IN_FLIGHT = 1 << 0,
|
|
|
38df8a |
|
|
|
38df8a |
|
|
|
38df8a |
- USBI_TRANSFER_CANCELLING = 1 << 2,
|
|
|
38df8a |
+ USBI_TRANSFER_CANCELLING = 1 << 1,
|
|
|
38df8a |
|
|
|
38df8a |
|
|
|
38df8a |
- USBI_TRANSFER_DEVICE_DISAPPEARED = 1 << 3,
|
|
|
38df8a |
-
|
|
|
38df8a |
- /* Transfer is currently being submitted */
|
|
|
38df8a |
- USBI_TRANSFER_SUBMITTING = 1 << 4,
|
|
|
38df8a |
-
|
|
|
38df8a |
- /* Transfer successfully submitted by backend */
|
|
|
38df8a |
- USBI_TRANSFER_IN_FLIGHT = 1 << 5,
|
|
|
38df8a |
+ USBI_TRANSFER_DEVICE_DISAPPEARED = 1 << 2,
|
|
|
38df8a |
+};
|
|
|
38df8a |
|
|
|
38df8a |
- /* Completion handler has run */
|
|
|
38df8a |
- USBI_TRANSFER_COMPLETED = 1 << 6,
|
|
|
38df8a |
+enum usbi_transfer_timeout_flags {
|
|
|
38df8a |
+
|
|
|
38df8a |
+ USBI_TRANSFER_OS_HANDLES_TIMEOUT = 1 << 0,
|
|
|
38df8a |
|
|
|
38df8a |
|
|
|
38df8a |
- USBI_TRANSFER_TIMEOUT_HANDLED = 1 << 7,
|
|
|
38df8a |
+ USBI_TRANSFER_TIMEOUT_HANDLED = 1 << 1,
|
|
|
38df8a |
+
|
|
|
38df8a |
+ /* The transfer timeout was successfully processed */
|
|
|
38df8a |
+ USBI_TRANSFER_TIMED_OUT = 1 << 2,
|
|
|
38df8a |
};
|
|
|
38df8a |
|
|
|
38df8a |
#define USBI_TRANSFER_TO_LIBUSB_TRANSFER(transfer) \
|
|
|
38df8a |
diff
|
|
|
38df8a |
index 6794509..736bcc1 100644
|
|
|
38df8a |
|
|
|
38df8a |
+++ b/libusb/os/darwin_usb.c
|
|
|
38df8a |
@@ -1471,7 +1471,7 @@ static int submit_bulk_transfer(struct usbi_transfer *itransfer) {
|
|
|
38df8a |
ret = (*(cInterface->interface))->WritePipeAsync(cInterface->interface, pipeRef, transfer->buffer,
|
|
|
38df8a |
transfer->length, darwin_async_io_callback, itransfer);
|
|
|
38df8a |
} else {
|
|
|
38df8a |
- itransfer->flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT;
|
|
|
38df8a |
+ itransfer->timeout_flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT;
|
|
|
38df8a |
|
|
|
38df8a |
if (IS_XFERIN(transfer))
|
|
|
38df8a |
ret = (*(cInterface->interface))->ReadPipeAsyncTO(cInterface->interface, pipeRef, transfer->buffer,
|
|
|
38df8a |
@@ -1503,7 +1503,7 @@ static int submit_stream_transfer(struct usbi_transfer *itransfer) {
|
|
|
38df8a |
return LIBUSB_ERROR_NOT_FOUND;
|
|
|
38df8a |
}
|
|
|
38df8a |
|
|
|
38df8a |
- itransfer->flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT;
|
|
|
38df8a |
+ itransfer->timeout_flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT;
|
|
|
38df8a |
|
|
|
38df8a |
if (IS_XFERIN(transfer))
|
|
|
38df8a |
ret = (*(cInterface->interface))->ReadStreamsPipeAsyncTO(cInterface->interface, pipeRef, itransfer->stream_id,
|
|
|
38df8a |
@@ -1631,7 +1631,7 @@ static int submit_control_transfer(struct usbi_transfer *itransfer) {
|
|
|
38df8a |
tpriv->req.completionTimeout = transfer->timeout;
|
|
|
38df8a |
tpriv->req.noDataTimeout = transfer->timeout;
|
|
|
38df8a |
|
|
|
38df8a |
- itransfer->flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT;
|
|
|
38df8a |
+ itransfer->timeout_flags |= USBI_TRANSFER_OS_HANDLES_TIMEOUT;
|
|
|
38df8a |
|
|
|
38df8a |
|
|
|
38df8a |
|
|
|
38df8a |
@@ -1780,7 +1780,7 @@ static void darwin_async_io_callback (void *refcon, IOReturn result, void *arg0)
|
|
|
38df8a |
}
|
|
|
38df8a |
|
|
|
38df8a |
static int darwin_transfer_status (struct usbi_transfer *itransfer, kern_return_t result) {
|
|
|
38df8a |
- if (itransfer->flags & USBI_TRANSFER_TIMED_OUT)
|
|
|
38df8a |
+ if (itransfer->timeout_flags & USBI_TRANSFER_TIMED_OUT)
|
|
|
38df8a |
result = kIOUSBTransactionTimeout;
|
|
|
38df8a |
|
|
|
38df8a |
switch (result) {
|
|
|
38df8a |
@@ -1797,7 +1797,7 @@ static int darwin_transfer_status (struct usbi_transfer *itransfer, kern_return_
|
|
|
38df8a |
return LIBUSB_TRANSFER_OVERFLOW;
|
|
|
38df8a |
case kIOUSBTransactionTimeout:
|
|
|
38df8a |
usbi_warn (ITRANSFER_CTX (itransfer), "transfer error: timed out");
|
|
|
38df8a |
- itransfer->flags |= USBI_TRANSFER_TIMED_OUT;
|
|
|
38df8a |
+ itransfer->timeout_flags |= USBI_TRANSFER_TIMED_OUT;
|
|
|
38df8a |
return LIBUSB_TRANSFER_TIMED_OUT;
|
|
|
38df8a |
default:
|
|
|
38df8a |
usbi_warn (ITRANSFER_CTX (itransfer), "transfer error: %s (value = 0x%08x)", darwin_error_str (result), result);
|
|
|
38df8a |
diff
|
|
|
38df8a |
index ea79210..3adf53e 100644
|
|
|
38df8a |
|
|
|
38df8a |
|
|
|
38df8a |
@@ -1 +1 @@
|
|
|
38df8a |
-#define LIBUSB_NANO 11006
|
|
|
38df8a |
+#define LIBUSB_NANO 11007
|
|
|
38df8a |
--
|
|
|
38df8a |
2.7.4
|
|
|
38df8a |
|