After cancelling transfers, wait for all handling to complete.

Calling libusb_cancel_transfer only starts the cancellation of a
transfer. The process is not complete until the transfer callback
has been called with status LIBUSB_TRANSFER_CANCELLED.

If hackrf_start_rx() is called soon after hackrf_stop_rx(),
prepare_transfers() may be called before the previous cancellations
are completed, resulting in a LIBUSB_ERROR_BUSY when a transfer is
reused with libusb_submit_transfer().

To prevent this happening, we keep track of which transfers have
finished (either by completion, or cancellation), and make
cancel_transfers() wait until all transfers are finished.

This is implemented using a pthread condition variable which is
signalled from the transfer thread.
This commit is contained in:
Martin Ling
2022-01-07 21:55:27 +00:00
parent 837b5ee9c8
commit 4c9fcf8665

View File

@ -127,6 +127,10 @@ struct hackrf_device {
unsigned char buffer[TRANSFER_COUNT * TRANSFER_BUFFER_SIZE]; unsigned char buffer[TRANSFER_COUNT * TRANSFER_BUFFER_SIZE];
bool transfers_setup; /* true if the USB transfers have been setup */ bool transfers_setup; /* true if the USB transfers have been setup */
pthread_mutex_t transfer_lock; /* must be held to cancel or restart transfers */ pthread_mutex_t transfer_lock; /* must be held to cancel or restart transfers */
bool transfer_finished[TRANSFER_COUNT]; /* which transfers have finished */
volatile bool all_finished; /* whether all transfers have finished */
pthread_cond_t all_finished_cv; /* signalled when all transfers have finished */
pthread_mutex_t all_finished_lock; /* used to protect all_finished */
}; };
typedef struct { typedef struct {
@ -205,6 +209,7 @@ static int transfers_check_setup(hackrf_device* device)
static int cancel_transfers(hackrf_device* device) static int cancel_transfers(hackrf_device* device)
{ {
uint32_t transfer_index; uint32_t transfer_index;
int i;
if(transfers_check_setup(device) == true) if(transfers_check_setup(device) == true)
{ {
@ -230,6 +235,20 @@ static int cancel_transfers(hackrf_device* device)
// is set to false. // is set to false.
pthread_mutex_unlock(&device->transfer_lock); pthread_mutex_unlock(&device->transfer_lock);
// Now wait for the transfer thread to signal that all transfers
// have finished, either by completing or being fully cancelled.
pthread_mutex_lock(&device->all_finished_lock);
while (!device->all_finished) {
pthread_cond_wait(&device->all_finished_cv, &device->all_finished_lock);
}
pthread_mutex_unlock(&device->all_finished_lock);
// Now that all waiting and handling is completed, it's safe to
// reset these flags ready for the next time.
for (i = 0; i < TRANSFER_COUNT; i++)
device->transfer_finished[i] = false;
device->all_finished = false;
return HACKRF_SUCCESS; return HACKRF_SUCCESS;
} else { } else {
return HACKRF_ERROR_OTHER; return HACKRF_ERROR_OTHER;
@ -592,7 +611,7 @@ libusb_device_handle* hackrf_open_usb(const char* const desired_serial_number)
static int hackrf_open_setup(libusb_device_handle* usb_device, hackrf_device** device) static int hackrf_open_setup(libusb_device_handle* usb_device, hackrf_device** device)
{ {
int result; int result, i;
hackrf_device* lib_device; hackrf_device* lib_device;
//int speed = libusb_get_device_speed(usb_device); //int speed = libusb_get_device_speed(usb_device);
@ -628,6 +647,9 @@ static int hackrf_open_setup(libusb_device_handle* usb_device, hackrf_device** d
lib_device->transfer_thread_started = false; lib_device->transfer_thread_started = false;
lib_device->streaming = false; lib_device->streaming = false;
lib_device->do_exit = false; lib_device->do_exit = false;
for (i = 0; i < TRANSFER_COUNT; i++)
lib_device->transfer_finished[i] = false;
lib_device->all_finished = false;
result = pthread_mutex_init(&lib_device->transfer_lock, NULL); result = pthread_mutex_init(&lib_device->transfer_lock, NULL);
if( result != 0 ) if( result != 0 )
@ -638,6 +660,24 @@ static int hackrf_open_setup(libusb_device_handle* usb_device, hackrf_device** d
return HACKRF_ERROR_THREAD; return HACKRF_ERROR_THREAD;
} }
result = pthread_mutex_init(&lib_device->all_finished_lock, NULL);
if( result != 0 )
{
free(lib_device);
libusb_release_interface(usb_device, 0);
libusb_close(usb_device);
return HACKRF_ERROR_THREAD;
}
result = pthread_cond_init(&lib_device->all_finished_cv, NULL);
if( result != 0 )
{
free(lib_device);
libusb_release_interface(usb_device, 0);
libusb_close(usb_device);
return HACKRF_ERROR_THREAD;
}
result = allocate_transfers(lib_device); result = allocate_transfers(lib_device);
if( result != 0 ) if( result != 0 )
{ {
@ -1566,9 +1606,32 @@ static void* transfer_threadproc(void* arg)
return NULL; return NULL;
} }
static void transfer_finished(struct hackrf_device* device, struct libusb_transfer* finished_transfer)
{
int i;
bool all_finished = true;
for (i = 0; i < TRANSFER_COUNT; i++) {
if (device->transfers[i] == finished_transfer) {
device->transfer_finished[i] = true;
} else {
all_finished &= device->transfer_finished[i];
}
}
if (all_finished) {
pthread_mutex_lock(&device->all_finished_lock);
device->all_finished = true;
pthread_cond_signal(&device->all_finished_cv);
pthread_mutex_unlock(&device->all_finished_lock);
}
}
static void LIBUSB_CALL hackrf_libusb_transfer_callback(struct libusb_transfer* usb_transfer) static void LIBUSB_CALL hackrf_libusb_transfer_callback(struct libusb_transfer* usb_transfer)
{ {
hackrf_device* device = (hackrf_device*)usb_transfer->user_data; hackrf_device* device = (hackrf_device*)usb_transfer->user_data;
bool resubmit;
int result;
if(usb_transfer->status == LIBUSB_TRANSFER_COMPLETED) if(usb_transfer->status == LIBUSB_TRANSFER_COMPLETED)
{ {
@ -1588,8 +1651,7 @@ static void LIBUSB_CALL hackrf_libusb_transfer_callback(struct libusb_transfer*
// of stopping them. // of stopping them.
pthread_mutex_lock(&device->transfer_lock); pthread_mutex_lock(&device->transfer_lock);
int result = 0; if ((resubmit = device->transfers_setup)) {
if( device->transfers_setup ) {
result = libusb_submit_transfer(usb_transfer); result = libusb_submit_transfer(usb_transfer);
} }
@ -1597,17 +1659,14 @@ static void LIBUSB_CALL hackrf_libusb_transfer_callback(struct libusb_transfer*
// cancelled or restarted, not both. // cancelled or restarted, not both.
pthread_mutex_unlock(&device->transfer_lock); pthread_mutex_unlock(&device->transfer_lock);
if( result < 0) if (!resubmit || result < 0) {
{ transfer_finished(device, usb_transfer);
request_exit(device);
}else {
return;
} }
}else { } else {
request_exit(device); transfer_finished(device, usb_transfer);
} }
} else if(usb_transfer->status == LIBUSB_TRANSFER_CANCELLED) { } else if(usb_transfer->status == LIBUSB_TRANSFER_CANCELLED) {
/* Nothing; this will happen during shutdown */ transfer_finished(device, usb_transfer);
} else { } else {
/* Other cases LIBUSB_TRANSFER_NO_DEVICE /* Other cases LIBUSB_TRANSFER_NO_DEVICE
LIBUSB_TRANSFER_ERROR, LIBUSB_TRANSFER_TIMED_OUT LIBUSB_TRANSFER_ERROR, LIBUSB_TRANSFER_TIMED_OUT
@ -1626,21 +1685,14 @@ static int kill_transfer_thread(hackrf_device* device)
if( device->transfer_thread_started != false ) if( device->transfer_thread_started != false )
{ {
/* /*
* Schedule cancelling transfers before halting the * Cancel transfers. This call will block until the transfer
* libusb thread. This should result in the transfers * thread has handled all completion callbacks.
* being properly marked as cancelled.
*
* Ideally this would wait for the cancellations to
* complete with the callback but for now that
* isn't super easy to do.
*/ */
cancel_transfers(device); cancel_transfers(device);
/* /*
* Now call request_exit() to halt the main loop. * Now call request_exit() to halt the main loop.
*/ */
request_exit(device); request_exit(device);
value = NULL; value = NULL;
result = pthread_join(device->transfer_thread, &value); result = pthread_join(device->transfer_thread, &value);
if( result != 0 ) if( result != 0 )
@ -1687,12 +1739,15 @@ static int prepare_setup_transfers(hackrf_device* device,
static int create_transfer_thread(hackrf_device* device) static int create_transfer_thread(hackrf_device* device)
{ {
int result; int result, i;
if( device->transfer_thread_started == false ) if( device->transfer_thread_started == false )
{ {
device->streaming = false; device->streaming = false;
device->do_exit = false; device->do_exit = false;
for (i = 0; i < TRANSFER_COUNT; i++)
device->transfer_finished[i] = false;
device->all_finished = false;
result = pthread_create(&device->transfer_thread, 0, transfer_threadproc, device); result = pthread_create(&device->transfer_thread, 0, transfer_threadproc, device);
if( result == 0 ) if( result == 0 )
{ {
@ -1860,6 +1915,8 @@ int ADDCALL hackrf_close(hackrf_device* device)
free_transfers(device); free_transfers(device);
pthread_mutex_destroy(&device->transfer_lock); pthread_mutex_destroy(&device->transfer_lock);
pthread_cond_destroy(&device->all_finished_cv);
pthread_mutex_destroy(&device->all_finished_lock);
free(device); free(device);
} }