libusb transfers not properly cleaned up if device is unplugged while streaming

[johanhedin]

If the device is unplugged while streaming, the while-loop in transfer_threadproc exits and the calls to libusb_handle_events_timeout_completed stops. This will in most cases lead to non-completed and/or failed transfers to be left hanging around inside the libusb.

When the libusb context is then closed, these hanging transfers cause memory leaks.

As far as I understand libusb, if you have submitted a transfer with libusb_submit_transfer, you must continue to make calls to a libusb_handle_events_* function until a event callback takes place for the transfer in question.

Canceling a transfer does not change this, you still need to make calls to libusb_handle_events_* after canceling the transfer and wait for the callback to fire before the transfer is considered complete.

Actually, even during a normal stop of libairspy (a call to libairspy_stop_rx), transfers are hanging around unhandled but are "released" due to the call to airspy_set_receiver_mode to turn off the receiver. This call will issue a libusb_control_transfer that most likely call a libusb_handle_events_* function under the hood making a proper cleanup. This seem more like a unintended behavior then by design though.

A simple fix might be to make a call to libusb_handle_events_timeout inside kill_io_threads just after the join of the two threads. This will fire event callbacks for the remaining transfers and free internal libusb memory.

The calls to the callback is safe here since device->stop_requested is set to true and the callback will just be a no-op.

Another way (perhaps even more robust?) is to introduce a reference counter that keep track of transfers in flight. Increase the counter for every call to libusb_submit_transfer and decrease it in the callback. And then in kill_io_threads you just loop over calls to libusb_handle_events_timeout until the reference counter reaches zero.

All this is observed on Fedora 33 x86_64 with libairspy master and libusb 1.0.24 and using a Airspy R2.

Simple program to show the memory leak:

#include <stdio.h>
#include <unistd.h>
#include <inttypes.h>

#include <airspy.h>

struct ctx {
    unsigned counter;
};

static int data_cb(airspy_transfer *transfer) {
    struct ctx *ctx = (struct ctx*)transfer->ctx;

    if (++ctx->counter == 20) {
        printf("Consumer thread: Data received...\n");
        ctx->counter = 0;
    }

    return 0;
}

int main(int argc, char **argv) {
    uint64_t              serial = 0;
    struct airspy_device *dev = NULL;
    int                   ret;
    struct ctx            ctx = { 0 };

    // Get serial if provided
    if (argc > 1) {
        ret = sscanf(argv[1], "%" SCNx64, &serial);
        if (ret != 1) serial = 0;
    }

    printf("Main thread: Running memleak test using device %.16" PRIx64 "....\n", serial);

    ret = airspy_open_sn(&dev, serial);
    if (ret != AIRSPY_SUCCESS) {
        printf("Error: Unable to open device.\n");
        return 1;
    }

    airspy_start_rx(dev, data_cb, &ctx);

    printf("Main tread: Waiting for device to be unplugged...\n");
    while (airspy_is_streaming(dev) == AIRSPY_TRUE) {
        sleep(1);
    }

    airspy_stop_rx(dev);
    airspy_close(dev);

    return 0;
}

Compile and run with valgrind:

  $ gcc -Wall -I/usr/include/libairspy -o memleak memleak.c -lairspy -lusb-1.0 -lpthread
  $ valgrind --leak-check=full --show-leak-kinds=all --show-error-list=yes ./memleak

Unplugg the device and valgrind will most of the time indicate memory leaks in like three out of six tries.

[bvernoux]

Fixed with merge of #78