hidapi icon indicating copy to clipboard operation
hidapi copied to clipboard

hidapi-libusb: if `libusb_submit_transfer` fails in read_thread, `hid_close` hangs

Open thesilvanator opened this issue 3 years ago • 9 comments

I have a usb device (that is finicky with its endpoints, it identifies itself as a hid_device and responds to hid requests, but is missing an IN interrupt endpoint, so Linux does not recognize it as HID) that I open using hid_libusb_wrap_sys_device with a linux file descriptor. I can send and get feature reports just fine; however, on hid_close, it hangs: specifically in checking for the transfer loop to finish (hidapi/libusb/hid.c:1001) . I ran with the env variable LIBUSB_DEBUG=4 and saw that the call to submit_bulk_transfer in libusb fails.

I'm not sure how this interacts with the read_callback, but dev->transfer_loop_finished never gets set to 1 and libusb_handle_events_completed(usb_context, &dev->transfer_loop_finished); takes the full 60 seconds to complete before restarting the while loop.

thesilvanator avatar Sep 12 '22 18:09 thesilvanator

Interesting, I've never tested a devices w/o interrupt IN endpoint.

Can you build HIDAPI with DEBUG_PRINTF def, and share the logs? I'm curios to see are there any failures inside read_callback or read_thread functions, and where.

Youw avatar Sep 12 '22 20:09 Youw

I've previously just been using hidapi from my OS's package manager. Built it now, linked it an ran it, but it doesn't output anything to stderr like the LOG macro defines. I built with CMAKE_BUILD_TYPE=Debug and inserted a #define DEBUG_PRINTF right above the #ifdef DEBUG_PRINTF. Not sure if I'm doing something incorrectly for the build. Let me know if I am and I'll fix it asap and report back.

thesilvanator avatar Sep 12 '22 22:09 thesilvanator

So you built it. Have you made sure your application is linked against the built version of HIDAPI?

Youw avatar Sep 13 '22 10:09 Youw

Yep. ldd says I am linked against it and just to make sure I put a test printf("hello world") in hidapi_initialize_device and it displayed.

I ran it through gdb as well and it never gets to a branch where the LOG macro runs.

thesilvanator avatar Sep 13 '22 13:09 thesilvanator

Thanks, that is a good finding too.

One more thing:

but is missing an IN interrupt endpoint, so Linux does not recognize it as HID

Can you elaborate? Are you saying that hid_open/hid_open_path fails for that particular device?

Youw avatar Sep 13 '22 17:09 Youw

Yes, that is exactly what happens. We suspect the manufacturer made a mistake because the device/interface identifies itself as HID in the configuration descriptor and responds as intented to hid_send_feature_report and hid_get_feature_report.

I understand that because this device technicially doesn't meet the HID requirements by USB, this may not really be an issue for HIDAPI to fix. But I figured that hid_close stalling when the libusb_submit_transfer fails may be something you want to take a look at.

Additionally, I did some testing and setting dev->shutdown_thread = 1 and dev->transfer_loop_finished = 1 after libusb_submit_transfer fails seems to solve the problem. However, I'm not too familiar with the hidapi codebase so idk what the reprecussions of this change may be.

thesilvanator avatar Sep 13 '22 18:09 thesilvanator

Yes, that is exactly what happens.

I'm trying to get sense of what exactly fails. Being "finicky with its endpoints" is not enough for hid_open_path to fail. AFAIK the only thing that can fail in hid_open_path is if the interface class is not LIBUSB_CLASS_HID. In which case, the statement "device/interface identifies itself as HID" is not correct. The device does not identifies itself as HID, but it acts like HID, responds to HID protocol (at least some of it), etc.

Additionally, I did some testing and setting dev->shutdown_thread = 1 and dev->transfer_loop_finished = 1 after libusb_submit_transfer fails seems to solve the problem. However, I'm not too familiar with the hidapi codebase so idk what the reprecussions of this change may be.

If you mean checking this instance of libusb_submit_transfer - please do make a PR, lets review it. It might be exactly the fix.

Youw avatar Sep 13 '22 18:09 Youw

Sorry, there was a mistake on my part.

hid_open/hid_open_path fails on linux when using hidapi-hidraw. Because the interface is missing an IN interrupt endpoint, Linux never adds it to the usbhid driver, and hid_open/hid_open_path fail.

However, hid_open/hid_open_path does work on hidapi-libusb (although it still does hang on hid_close).

I can put together a PR and send it in the next couple hours.

thesilvanator avatar Sep 13 '22 18:09 thesilvanator

That makes better sense, thanks.

Youw avatar Sep 13 '22 18:09 Youw