Skip to content

Commit

Permalink
Fix InterruptStop if called before InterruptRead
Browse files Browse the repository at this point in the history
The goal is to make the polling termination (via
TAG_IFD_STOP_POLLING_THREAD) actually work even when it's
triggered before the InterruptRead has a chance to create and
publish the polling_transfer.

Without this patch, in some cases the termination was blocked for
10 minutes, because the thread that does InterruptStop might still
see the null polling_transfer meanwhile the other thread is about
to start this transfer. The proposed solution is to remember in a
separate (atomic) boolean flag whether the termination started, to
avoid this kind of race condition.
  • Loading branch information
emaxx-google authored and LudovicRousseau committed Sep 27, 2023
1 parent 8b60004 commit eb7d606
Showing 1 changed file with 17 additions and 0 deletions.
17 changes: 17 additions & 0 deletions src/ccid_usb.c
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ typedef struct
*/
_ccid_descriptor ccid;

/* whether the polling should be terminated */
_Atomic bool terminated;

/* libusb transfer for the polling (or NULL) */
_Atomic (struct libusb_transfer *) polling_transfer;

Expand Down Expand Up @@ -741,6 +744,7 @@ status_t OpenUSBByName(unsigned int reader_index, /*@null@*/ char *device)
usbDevice[reader_index].interface = interface;
usbDevice[reader_index].real_nb_opened_slots = 1;
usbDevice[reader_index].nb_opened_slots = &usbDevice[reader_index].real_nb_opened_slots;
atomic_init(&usbDevice[reader_index].terminated, false);
atomic_init(&usbDevice[reader_index].polling_transfer, NULL);
usbDevice[reader_index].disconnected = false;

Expand Down Expand Up @@ -1526,6 +1530,14 @@ int InterruptRead(int reader_index, int timeout /* in ms */)

atomic_store(&usbDevice[reader_index].polling_transfer, transfer);

// The termination might've been requested by the other thread before the
// polling_transfer field was written. In that case, we have to cancel the
// transfer here as opposed to InterruptStop().
bool terminated = atomic_load(&usbDevice[reader_index].terminated);
if (terminated) {
libusb_cancel_transfer(transfer);
}

while (!completed)
{
ret = libusb_handle_events_completed(ctx, &completed);
Expand Down Expand Up @@ -1590,6 +1602,11 @@ void InterruptStop(int reader_index)
return;
}

// Set the termination flag to handle the case in which the polling_transfer
// value read below is null. The order of operations is important, and it has
// to be opposite to the one in InterruptRead() to avoid race conditions.
atomic_store(&usbDevice[reader_index].terminated, true);

transfer = atomic_exchange(&usbDevice[reader_index].polling_transfer,
NULL);
if (transfer)
Expand Down

0 comments on commit eb7d606

Please sign in to comment.