Crash in reads with short timeout -- bug in _usb_cancel_io()
mcuee opened this issue · 1 comments
https://sourceforge.net/p/libusb-win32/mailman/message/36291183/
When calling e.g. usb_interrupt_read() that times out it will call:
- usb_interrupt_read
- _usb_transfer_sync
- usb_reap_async
- _usb_reap_async
- _usb_cancel_io
This is where it goes south. To cancel the I/O operation it calls:
_usb_abort_ep() to send the LIBUSB_IOCTL_ABORT_ENDPOINT to the driver
synchroniously. Even though the LIBUSB_IOCTL_ABORT_ENDPOINT is sent
synchroniously to the driver, the driver just forwards the request to
"usbd" further down the IRP chain as a "reset endpoint" request, which
has asynchrinous elements to it. So all-in-all you cannot regard the
request as cancelled when returning from _usb_abort_ep().
To fix this it was introduced that using WaitForSingleObject() would be
used to wait for the cancel to go through.
The first revision that had this was:
It was later reformatted into being a part of _usb_cancel_io().
However, WaitForSingleObject() is not called correctly. If you want to
be sure that the cancel has been executed, you must call
WaitForSingleObject() with a timeout that is large enough to ensure
that the request is processed. In many cases on a loaded system, this
ends tragically.
Potential fix
Let's check the return code. What about this:
static int _usb_cancel_io(usb_context_t *context)
{
int ret;
ret = _usb_abort_ep(context->dev, context->req.endpoint.endpoint);
if (ret == 0)
{
WaitForSingleObject(context->ol.hEvent, 1000);
}
else
{
WaitForSingleObject(context->ol.hEvent, 0);
}
return ret;
}
Use 1000 ms I think. That should always be plenty and it will keep it
from dead locking.
Regards,
Travis