Skip to content

Commit

Permalink
After cancelling transfers, wait for all handling to complete.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@martinling
martinling committed Jan 11, 2022
1 parent b346790 commit 9182f1c
Showing 1 changed file with 78 additions and 21 deletions.
99 changes: 78 additions & 21 deletions host/libhackrf/src/hackrf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ struct hackrf_device {
unsigned char buffer[TRANSFER_COUNT * TRANSFER_BUFFER_SIZE];
bool transfers_setup; /* true if the USB transfers have been setup */
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 {
Expand Down Expand Up @@ -205,6 +209,7 @@ static int transfers_check_setup(hackrf_device* device)
static int cancel_transfers(hackrf_device* device)
{
uint32_t transfer_index;
int i;

if(transfers_check_setup(device) == true)
{
Expand All @@ -230,6 +235,20 @@ static int cancel_transfers(hackrf_device* device)
// is set to false.
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;
} else {
return HACKRF_ERROR_OTHER;
Expand Down Expand Up @@ -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)
{
int result;
int result, i;
hackrf_device* lib_device;

//int speed = libusb_get_device_speed(usb_device);
Expand Down Expand Up @@ -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->streaming = 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);
if( result != 0 )
Expand All @@ -638,6 +660,24 @@ static int hackrf_open_setup(libusb_device_handle* usb_device, hackrf_device** d
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);
if( result != 0 )
{
Expand Down Expand Up @@ -1566,9 +1606,32 @@ static void* transfer_threadproc(void* arg)
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)
{
hackrf_device* device = (hackrf_device*)usb_transfer->user_data;
bool resubmit;
int result;

if(usb_transfer->status == LIBUSB_TRANSFER_COMPLETED)
{
Expand All @@ -1588,26 +1651,22 @@ static void LIBUSB_CALL hackrf_libusb_transfer_callback(struct libusb_transfer*
// of stopping them.
pthread_mutex_lock(&device->transfer_lock);

int result = 0;
if( device->transfers_setup ) {
if ((resubmit = device->transfers_setup)) {
result = libusb_submit_transfer(usb_transfer);
}

// Now we can release the lock. Our transfer was either
// cancelled or restarted, not both.
pthread_mutex_unlock(&device->transfer_lock);

if( result < 0)
{
request_exit(device);
}else {
return;
if (!resubmit || result < 0) {
transfer_finished(device, usb_transfer);
}
}else {
request_exit(device);
} else {
transfer_finished(device, usb_transfer);
}
} else if(usb_transfer->status == LIBUSB_TRANSFER_CANCELLED) {
/* Nothing; this will happen during shutdown */
transfer_finished(device, usb_transfer);
} else {
/* Other cases LIBUSB_TRANSFER_NO_DEVICE
LIBUSB_TRANSFER_ERROR, LIBUSB_TRANSFER_TIMED_OUT
Expand All @@ -1626,21 +1685,14 @@ static int kill_transfer_thread(hackrf_device* device)
if( device->transfer_thread_started != false )
{
/*
* Schedule cancelling transfers before halting the
* libusb thread. This should result in the transfers
* 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. This call will block until the transfer
* thread has handled all completion callbacks.
*/
cancel_transfers(device);

/*
* Now call request_exit() to halt the main loop.
*/
request_exit(device);

value = NULL;
result = pthread_join(device->transfer_thread, &value);
if( result != 0 )
Expand Down Expand Up @@ -1687,12 +1739,15 @@ static int prepare_setup_transfers(hackrf_device* device,

static int create_transfer_thread(hackrf_device* device)
{
int result;
int result, i;

if( device->transfer_thread_started == false )
{
device->streaming = 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);
if( result == 0 )
{
Expand Down Expand Up @@ -1860,6 +1915,8 @@ int ADDCALL hackrf_close(hackrf_device* device)
free_transfers(device);

pthread_mutex_destroy(&device->transfer_lock);
pthread_cond_destroy(&device->all_finished_cv);
pthread_mutex_destroy(&device->all_finished_lock);

free(device);
}
Expand Down

0 comments on commit 9182f1c

Please sign in to comment.