Windows: Filter out non-libusb I/O completions

The I/O completion port thread gets notified of every I/O operation that
completes for a given HANDLE, but not all I/O operations originate from
within libusb. For example, libusbK performs a number of I/O control
calls during initialization and specific device operations, such as
setting an interface alternate setting. These non-libusb operations need
to be ignored as the OVERLAPPED associated with such operations is not
tied to a libusb_transfer structure.

Resolve this situation by using the libusb device handle as the I/O
completion port's unique key and keeping a list of active transfers for
each device handle. When the I/O competion port thread is notified of an
I/O completion, it will first match the OVERLAPPED to an outstanding
transfer before acting on it.

Closes #844

Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>

diff --git a/libusb/os/windows_common.c b/libusb/os/windows_common.c
index efb746b..a5e2a7c 100644 (file)
--- a/libusb/os/windows_common.c
+++ b/libusb/os/windows_common.c
@@ -415,8 +415,11 @@ static unsigned __stdcall windows_iocp_thread(void *arg)
        DWORD num_bytes;
        ULONG_PTR completion_key;
        OVERLAPPED *overlapped;
+       struct libusb_device_handle *dev_handle;
+       struct windows_device_handle_priv *handle_priv;
        struct windows_transfer_priv *transfer_priv;
        struct usbi_transfer *itransfer;
+       bool found;
 
        usbi_dbg("I/O completion thread started");
 
@@ -434,7 +437,29 @@ static unsigned __stdcall windows_iocp_thread(void *arg)
                        break;
                }
 
-               transfer_priv = container_of(overlapped, struct windows_transfer_priv, overlapped);
+               // Find the transfer associated with the OVERLAPPED that just completed.
+               // If we cannot find a match, the I/O operation originated from outside of libusb
+               // (e.g. within libusbK) and we need to ignore it.
+               dev_handle = (struct libusb_device_handle *)completion_key;
+               handle_priv = usbi_get_device_handle_priv(dev_handle);
+               found = false;
+               usbi_mutex_lock(&dev_handle->lock);
+               list_for_each_entry(transfer_priv, &handle_priv->active_transfers, list, struct windows_transfer_priv) {
+                       if (overlapped == &transfer_priv->overlapped) {
+                               // This OVERLAPPED belongs to us, remove the transfer from the device handle's list
+                               list_del(&transfer_priv->list);
+                               found = true;
+                               break;
+                       }
+               }
+               usbi_mutex_unlock(&dev_handle->lock);
+
+               if (!found) {
+                       usbi_dbg("ignoring overlapped %p for handle %p (device %u.%u)",
+                               overlapped, dev_handle, dev_handle->dev->bus_number, dev_handle->dev->device_address);
+                       continue;
+               }
+
                itransfer = (struct usbi_transfer *)((unsigned char *)transfer_priv + PTR_ALIGN(sizeof(*transfer_priv)));
                usbi_dbg("transfer %p completed, length %lu",
                         USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer), ULONG_CAST(num_bytes));
@@ -613,6 +638,9 @@ static int windows_get_device_list(struct libusb_context *ctx, struct discovered
 static int windows_open(struct libusb_device_handle *dev_handle)
 {
        struct windows_context_priv *priv = usbi_get_context_priv(HANDLE_CTX(dev_handle));
+       struct windows_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
+
+       list_init(&handle_priv->active_transfers);
        return priv->backend->open(dev_handle);
 }
 
@@ -697,8 +725,10 @@ static void windows_destroy_device(struct libusb_device *dev)
 static int windows_submit_transfer(struct usbi_transfer *itransfer)
 {
        struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
-       struct libusb_context *ctx = TRANSFER_CTX(transfer);
+       struct libusb_device_handle *dev_handle = transfer->dev_handle;
+       struct libusb_context *ctx = HANDLE_CTX(dev_handle);
        struct windows_context_priv *priv = usbi_get_context_priv(ctx);
+       struct windows_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
        struct windows_transfer_priv *transfer_priv = usbi_get_transfer_priv(itransfer);
        int r;
 
@@ -721,8 +751,18 @@ static int windows_submit_transfer(struct usbi_transfer *itransfer)
                transfer_priv->handle = NULL;
        }
 
+       // Add transfer to the device handle's list
+       usbi_mutex_lock(&dev_handle->lock);
+       list_add_tail(&transfer_priv->list, &handle_priv->active_transfers);
+       usbi_mutex_unlock(&dev_handle->lock);
+
        r = priv->backend->submit_transfer(itransfer);
        if (r != LIBUSB_SUCCESS) {
+               // Remove the unsuccessful transfer from the device handle's list
+               usbi_mutex_lock(&dev_handle->lock);
+               list_del(&transfer_priv->list);
+               usbi_mutex_unlock(&dev_handle->lock);
+
                // Always call the backend's clear_transfer_priv() function on failure
                priv->backend->clear_transfer_priv(itransfer);
                transfer_priv->handle = NULL;
@@ -880,6 +920,6 @@ const struct usbi_os_backend usbi_backend = {
        windows_handle_transfer_completion,
        sizeof(struct windows_context_priv),
        sizeof(union windows_device_priv),
-       sizeof(union windows_device_handle_priv),
+       sizeof(struct windows_device_handle_priv),
        sizeof(struct windows_transfer_priv),
 };

diff --git a/libusb/os/windows_common.h b/libusb/os/windows_common.h
index d9958c6..c84f1f9 100644 (file)
--- a/libusb/os/windows_common.h
+++ b/libusb/os/windows_common.h
@@ -338,20 +338,36 @@ union windows_device_priv {
        struct winusb_device_priv winusb_priv;
 };
 
-union windows_device_handle_priv {
-       struct usbdk_device_handle_priv usbdk_priv;
-       struct winusb_device_handle_priv winusb_priv;
+struct windows_device_handle_priv {
+       struct list_head active_transfers;
+       union {
+               struct usbdk_device_handle_priv usbdk_priv;
+               struct winusb_device_handle_priv winusb_priv;
+       };
 };
 
 struct windows_transfer_priv {
        OVERLAPPED overlapped;
        HANDLE handle;
+       struct list_head list;
        union {
                struct usbdk_transfer_priv usbdk_priv;
                struct winusb_transfer_priv winusb_priv;
        };
 };
 
+static inline struct usbdk_device_handle_priv *get_usbdk_device_handle_priv(struct libusb_device_handle *dev_handle)
+{
+       struct windows_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
+       return &handle_priv->usbdk_priv;
+}
+
+static inline struct winusb_device_handle_priv *get_winusb_device_handle_priv(struct libusb_device_handle *dev_handle)
+{
+       struct windows_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
+       return &handle_priv->winusb_priv;
+}
+
 static inline OVERLAPPED *get_transfer_priv_overlapped(struct usbi_transfer *itransfer)
 {
        struct windows_transfer_priv *transfer_priv = usbi_get_transfer_priv(itransfer);

diff --git a/libusb/os/windows_usbdk.c b/libusb/os/windows_usbdk.c
index 76aba16..9f52b48 100644 (file)
--- a/libusb/os/windows_usbdk.c
+++ b/libusb/os/windows_usbdk.c
@@ -413,7 +413,7 @@ static int usbdk_open(struct libusb_device_handle *dev_handle)
 
        device_priv->system_handle = usbdk_helper.GetRedirectorSystemHandle(device_priv->redirector_handle);
 
-       if (CreateIoCompletionPort(device_priv->system_handle, priv->completion_port, 0, 0) == NULL) {
+       if (CreateIoCompletionPort(device_priv->system_handle, priv->completion_port, (ULONG_PTR)dev_handle, 0) == NULL) {
                usbi_err(ctx, "failed to associate handle to I/O completion port: %s", windows_error_str(0));
                usbdk_helper.StopRedirect(device_priv->redirector_handle);
                device_priv->system_handle = NULL;

diff --git a/libusb/os/windows_winusb.c b/libusb/os/windows_winusb.c
index ea51038..a370ede 100644 (file)
--- a/libusb/os/windows_winusb.c
+++ b/libusb/os/windows_winusb.c
@@ -488,9 +488,9 @@ static int get_interface_by_endpoint(struct libusb_config_descriptor *conf_desc,
 /*
  * Open a device and associate the HANDLE with the context's I/O completion port
  */
-static HANDLE windows_open(struct libusb_device *dev, const char *path, DWORD access)
+static HANDLE windows_open(struct libusb_device_handle *dev_handle, const char *path, DWORD access)
 {
-       struct libusb_context *ctx = DEVICE_CTX(dev);
+       struct libusb_context *ctx = HANDLE_CTX(dev_handle);
        struct windows_context_priv *priv = usbi_get_context_priv(ctx);
        HANDLE handle;
 
@@ -498,7 +498,7 @@ static HANDLE windows_open(struct libusb_device *dev, const char *path, DWORD ac
        if (handle == INVALID_HANDLE_VALUE)
                return handle;
 
-       if (CreateIoCompletionPort(handle, priv->completion_port, 0, 0) == NULL) {
+       if (CreateIoCompletionPort(handle, priv->completion_port, (ULONG_PTR)dev_handle, 0) == NULL) {
                usbi_err(ctx, "failed to associate handle to I/O completion port: %s", windows_error_str(0));
                CloseHandle(handle);
                return INVALID_HANDLE_VALUE;
@@ -593,7 +593,7 @@ static int get_sub_api(char *driver, int api)
 static int auto_claim(struct libusb_transfer *transfer, int *interface_number, int api_type)
 {
        struct winusb_device_handle_priv *handle_priv =
-               usbi_get_device_handle_priv(transfer->dev_handle);
+               get_winusb_device_handle_priv(transfer->dev_handle);
        struct winusb_device_priv *priv = usbi_get_device_priv(transfer->dev_handle->dev);
        int current_interface = *interface_number;
        int r = LIBUSB_SUCCESS;
@@ -640,7 +640,7 @@ static void auto_release(struct usbi_transfer *itransfer)
        struct winusb_transfer_priv *transfer_priv = get_winusb_transfer_priv(itransfer);
        struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
        libusb_device_handle *dev_handle = transfer->dev_handle;
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle);
        int r;
 
        usbi_mutex_lock(&autoclaim_lock);
@@ -2375,7 +2375,7 @@ static void winusbx_exit(void)
 static int winusbx_open(int sub_api, struct libusb_device_handle *dev_handle)
 {
        struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev);
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle);
        HANDLE file_handle;
        int i;
 
@@ -2385,7 +2385,7 @@ static int winusbx_open(int sub_api, struct libusb_device_handle *dev_handle)
        for (i = 0; i < USB_MAXINTERFACES; i++) {
                if ((priv->usb_interface[i].path != NULL)
                                && (priv->usb_interface[i].apib->id == USB_API_WINUSBX)) {
-                       file_handle = windows_open(dev_handle->dev, priv->usb_interface[i].path, GENERIC_READ | GENERIC_WRITE);
+                       file_handle = windows_open(dev_handle, priv->usb_interface[i].path, GENERIC_READ | GENERIC_WRITE);
                        if (file_handle == INVALID_HANDLE_VALUE) {
                                usbi_err(HANDLE_CTX(dev_handle), "could not open device %s (interface %d): %s", priv->usb_interface[i].path, i, windows_error_str(0));
                                switch (GetLastError()) {
@@ -2407,7 +2407,7 @@ static int winusbx_open(int sub_api, struct libusb_device_handle *dev_handle)
 
 static void winusbx_close(int sub_api, struct libusb_device_handle *dev_handle)
 {
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle);
        struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev);
        HANDLE handle;
        int i;
@@ -2452,7 +2452,7 @@ static void winusbx_close(int sub_api, struct libusb_device_handle *dev_handle)
 
 static int winusbx_configure_endpoints(int sub_api, struct libusb_device_handle *dev_handle, uint8_t iface)
 {
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle);
        struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev);
        HANDLE winusb_handle = handle_priv->interface_handle[iface].api_handle;
        UCHAR policy;
@@ -2506,7 +2506,7 @@ static int winusbx_configure_endpoints(int sub_api, struct libusb_device_handle
 static int winusbx_claim_interface(int sub_api, struct libusb_device_handle *dev_handle, uint8_t iface)
 {
        struct libusb_context *ctx = HANDLE_CTX(dev_handle);
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle);
        struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev);
        bool is_using_usbccgp = (priv->apib->id == USB_API_COMPOSITE);
        HDEVINFO dev_info;
@@ -2557,7 +2557,7 @@ static int winusbx_claim_interface(int sub_api, struct libusb_device_handle *dev
                                        *dev_interface_path_guid_start = '\0';
 
                                        if (strncmp(dev_interface_path, priv->usb_interface[iface].path, strlen(dev_interface_path)) == 0) {
-                                               file_handle = windows_open(dev_handle->dev, filter_path, GENERIC_READ | GENERIC_WRITE);
+                                               file_handle = windows_open(dev_handle, filter_path, GENERIC_READ | GENERIC_WRITE);
                                                if (file_handle != INVALID_HANDLE_VALUE) {
                                                        if (WinUSBX[sub_api].Initialize(file_handle, &winusb_handle)) {
                                                                // Replace the existing file handle with the working one
@@ -2622,7 +2622,7 @@ static int winusbx_claim_interface(int sub_api, struct libusb_device_handle *dev
 
 static int winusbx_release_interface(int sub_api, struct libusb_device_handle *dev_handle, uint8_t iface)
 {
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle);
        struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev);
        HANDLE winusb_handle;
 
@@ -2643,7 +2643,7 @@ static int winusbx_release_interface(int sub_api, struct libusb_device_handle *d
  */
 static int get_valid_interface(struct libusb_device_handle *dev_handle, int api_id)
 {
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle);
        struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev);
        int i;
 
@@ -2667,7 +2667,7 @@ static int get_valid_interface(struct libusb_device_handle *dev_handle, int api_
 */
 static int check_valid_interface(struct libusb_device_handle *dev_handle, unsigned short interface, int api_id)
 {
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle);
        struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev);
 
        if (interface >= USB_MAXINTERFACES)
@@ -2714,7 +2714,7 @@ static int winusbx_submit_control_transfer(int sub_api, struct usbi_transfer *it
        struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
        struct winusb_device_priv *priv = usbi_get_device_priv(transfer->dev_handle->dev);
        struct winusb_transfer_priv *transfer_priv = get_winusb_transfer_priv(itransfer);
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(transfer->dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(transfer->dev_handle);
        PWINUSB_SETUP_PACKET setup = (PWINUSB_SETUP_PACKET)transfer->buffer;
        ULONG size;
        HANDLE winusb_handle;
@@ -2770,7 +2770,7 @@ static int winusbx_submit_control_transfer(int sub_api, struct usbi_transfer *it
 
 static int winusbx_set_interface_altsetting(int sub_api, struct libusb_device_handle *dev_handle, uint8_t iface, uint8_t altsetting)
 {
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle);
        struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev);
        HANDLE winusb_handle;
 
@@ -2826,7 +2826,7 @@ static int winusbx_submit_iso_transfer(int sub_api, struct usbi_transfer *itrans
 {
        struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
        struct winusb_transfer_priv *transfer_priv = get_winusb_transfer_priv(itransfer);
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(transfer->dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(transfer->dev_handle);
        struct winusb_device_priv *priv = usbi_get_device_priv(transfer->dev_handle->dev);
        HANDLE winusb_handle;
        OVERLAPPED *overlapped;
@@ -3016,7 +3016,7 @@ static int winusbx_submit_bulk_transfer(int sub_api, struct usbi_transfer *itran
 {
        struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
        struct winusb_transfer_priv *transfer_priv = get_winusb_transfer_priv(itransfer);
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(transfer->dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(transfer->dev_handle);
        struct winusb_device_priv *priv = usbi_get_device_priv(transfer->dev_handle->dev);
        HANDLE winusb_handle;
        OVERLAPPED *overlapped;
@@ -3057,7 +3057,7 @@ static int winusbx_submit_bulk_transfer(int sub_api, struct usbi_transfer *itran
 
 static int winusbx_clear_halt(int sub_api, struct libusb_device_handle *dev_handle, unsigned char endpoint)
 {
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle);
        struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev);
        HANDLE winusb_handle;
        int current_interface;
@@ -3084,7 +3084,7 @@ static int winusbx_clear_halt(int sub_api, struct libusb_device_handle *dev_hand
 static int winusbx_cancel_transfer(int sub_api, struct usbi_transfer *itransfer)
 {
        struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(transfer->dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(transfer->dev_handle);
        struct winusb_transfer_priv *transfer_priv = get_winusb_transfer_priv(itransfer);
        struct winusb_device_priv *priv = usbi_get_device_priv(transfer->dev_handle->dev);
        int current_interface = transfer_priv->interface_number;
@@ -3114,7 +3114,7 @@ static int winusbx_cancel_transfer(int sub_api, struct usbi_transfer *itransfer)
 // TODO: (post hotplug): see if we can force eject the device and redetect it (reuse hotplug?)
 static int winusbx_reset_device(int sub_api, struct libusb_device_handle *dev_handle)
 {
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle);
        struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev);
        HANDLE winusb_handle;
        int i, j;
@@ -4339,7 +4339,7 @@ static int composite_submit_control_transfer(int sub_api, struct usbi_transfer *
 static int composite_submit_bulk_transfer(int sub_api, struct usbi_transfer *itransfer)
 {
        struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(transfer->dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(transfer->dev_handle);
        struct winusb_device_priv *priv = usbi_get_device_priv(transfer->dev_handle->dev);
        int current_interface;
 
@@ -4360,7 +4360,7 @@ static int composite_submit_bulk_transfer(int sub_api, struct usbi_transfer *itr
 static int composite_submit_iso_transfer(int sub_api, struct usbi_transfer *itransfer)
 {
        struct libusb_transfer *transfer = USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer);
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(transfer->dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(transfer->dev_handle);
        struct winusb_device_priv *priv = usbi_get_device_priv(transfer->dev_handle->dev);
        int current_interface;
 
@@ -4380,7 +4380,7 @@ static int composite_submit_iso_transfer(int sub_api, struct usbi_transfer *itra
 
 static int composite_clear_halt(int sub_api, struct libusb_device_handle *dev_handle, unsigned char endpoint)
 {
-       struct winusb_device_handle_priv *handle_priv = usbi_get_device_handle_priv(dev_handle);
+       struct winusb_device_handle_priv *handle_priv = get_winusb_device_handle_priv(dev_handle);
        struct winusb_device_priv *priv = usbi_get_device_priv(dev_handle->dev);
        int current_interface;
 

diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index b8e72d6..4227ce9 100644 (file)
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11598
+#define LIBUSB_NANO 11599