io: Change remove_from_flying_list() to not lock flying_transfers_lock itself

This is now symmetric with add_to_flying_list(), which was already
requiring that *callers* lock flying_transfers_lock. Updated the only
2 callers.

This also makes the code correspond better to the big comment about
locking made in 138b661f.

Also documented at the top of several functions when they require that
flying_transfers_lock already be held. Verified by code review that it's
actually the case.

This should not change any behaviour at all.

References #1410
This commit is contained in:
Sean McBride
2023-12-27 23:02:28 -05:00
committed by Tormod Volden
parent f9ae36b13f
commit 7719ae5632
3 changed files with 16 additions and 9 deletions

View File

@@ -1349,7 +1349,7 @@ void API_EXPORTED libusb_free_transfer(struct libusb_transfer *transfer)
/* iterates through the flying transfers, and rearms the timer based on the
* next upcoming timeout.
* must be called with flying_list locked.
* NB: flying_transfers_lock must be held when calling this.
* returns 0 on success or a LIBUSB_ERROR code on failure.
*/
#ifdef HAVE_OS_TIMER
@@ -1389,7 +1389,8 @@ static inline int arm_timer_for_next_timeout(struct libusb_context *ctx)
/* add a transfer to the (timeout-sorted) active transfers list.
* This function will return non 0 if fails to update the timer,
* in which case the transfer is *not* on the flying_transfers list. */
* in which case the transfer is *not* on the flying_transfers list.
* NB: flying_transfers_lock MUST be held when calling this. */
static int add_to_flying_list(struct usbi_transfer *itransfer)
{
struct usbi_transfer *cur;
@@ -1451,20 +1452,19 @@ out:
/* remove a transfer from the active transfers list.
* This function will *always* remove the transfer from the
* flying_transfers list. It will return a LIBUSB_ERROR code
* if it fails to update the timer for the next timeout. */
* if it fails to update the timer for the next timeout.
* NB: flying_transfers_lock MUST be held when calling this. */
static int remove_from_flying_list(struct usbi_transfer *itransfer)
{
struct libusb_context *ctx = ITRANSFER_CTX(itransfer);
int rearm_timer;
int r = 0;
usbi_mutex_lock(&ctx->flying_transfers_lock);
rearm_timer = (TIMESPEC_IS_SET(&itransfer->timeout) &&
list_first_entry(&ctx->flying_transfers, struct usbi_transfer, list) == itransfer);
list_del(&itransfer->list);
if (rearm_timer)
r = arm_timer_for_next_timeout(ctx);
usbi_mutex_unlock(&ctx->flying_transfers_lock);
return r;
}
@@ -1558,8 +1558,11 @@ int API_EXPORTED libusb_submit_transfer(struct libusb_transfer *transfer)
}
usbi_mutex_unlock(&itransfer->lock);
if (r != LIBUSB_SUCCESS)
if (r != LIBUSB_SUCCESS) {
usbi_mutex_lock(&ctx->flying_transfers_lock);
remove_from_flying_list(itransfer);
usbi_mutex_unlock(&ctx->flying_transfers_lock);
}
return r;
}
@@ -1687,7 +1690,9 @@ int usbi_handle_transfer_completion(struct usbi_transfer *itransfer,
uint8_t flags;
int r;
usbi_mutex_lock(&ctx->flying_transfers_lock);
r = remove_from_flying_list(itransfer);
usbi_mutex_unlock(&ctx->flying_transfers_lock);
if (r < 0)
usbi_err(ctx, "failed to set timer for next timeout");
@@ -2039,6 +2044,7 @@ int API_EXPORTED libusb_wait_for_event(libusb_context *ctx, struct timeval *tv)
return 0;
}
// NB: flying_transfers_lock must be held when calling this
static void handle_timeout(struct usbi_transfer *itransfer)
{
struct libusb_transfer *transfer =
@@ -2054,6 +2060,7 @@ static void handle_timeout(struct usbi_transfer *itransfer)
"async cancel failed %d", r);
}
// NB: flying_transfers_lock must be held when calling this
static void handle_timeouts_locked(struct libusb_context *ctx)
{
struct timespec systime;

View File

@@ -380,7 +380,7 @@ struct libusb_context {
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;
usbi_mutex_t flying_transfers_lock; /* for flying_transfers and timeout_flags */
#if !defined(PLATFORM_WINDOWS)
/* user callbacks for pollfd changes */
@@ -578,7 +578,7 @@ struct usbi_transfer {
int transferred;
uint32_t stream_id;
uint32_t state_flags; /* Protected by usbi_transfer->lock */
uint32_t timeout_flags; /* Protected by the flying_stransfers_lock */
uint32_t timeout_flags; /* Protected by the flying_transfers_lock */
/* The device reference is held until destruction for logging
* even after dev_handle is set to NULL. */

View File

@@ -1 +1 @@
#define LIBUSB_NANO 11865
#define LIBUSB_NANO 11866