search

signal11 / hidapi

A Simple library for communicating with USB and Bluetooth HID devices on Linux, Mac, and Windows.
http://www.signal11.us/oss/hidapi/
Other
2.46k stars 901 forks source link

hid_close does not abort hid_read #48

Open vanweric opened 12 years ago

vanweric commented 12 years ago

I am calling hid_read blocking in one thread, and call hid_close in another thread. I'd expect that hid_read would return, but it continues to block indefinitely.

It looks like it might be stuck in GetOverlappedResult, but I thought CancelIO would have handled that.

Are you able to reproduce this?

vanweric commented 12 years ago

My apologies, I failed to mention that I was running on Windows 7 x64.

I changed CancelIO to CancelIoEx(dev->device_handle,NULL), and the blocking call to hid_read is aborting as expected. I also tested this with a call to hid_read_timeout with a very long timeout period, and it is also appropriately aborting.

signal11 commented 12 years ago

Unfortunately that function only works in Vista and newer, not on XP. It's definitely something nice to have in there. Maybe we could dynamically look it up, and call if it exists.

vanweric commented 12 years ago

That is unfortunate: I need to support XP SP3 and later. I think I could cope with the limitations of the dynamic lookup if push came to shove, but I'd still really like to be able to safely call hid_close from an arbitrary thread when I have a read pending.

I haven't tried this yet, but could we signal the event before it is closed? If so, it could still call WaitForSingleObject with milliseconds set to INFINITE. I'm not clear on how to guarantee that it then returns -1. Does closing the device_handle do this, or would there need to be a flag of some sort?

signal11 commented 12 years ago

Yes, unfortunate. The other implementations (at least linux-libusb and mac) do support this behavior because all the threading is controlled by me, and I can manually release all the mutexes which blocking read()s are waiting on. Windows and Linux-hidraw use the native system read() calls, and are subject to their platform-dependent behavior.

That said, from the documentation I've read, calling close() and a handle which another thread is blocking in a read() call on is undefined behavior in Unix. Of course it's all dependent on how the driver underneath is implemented. Unfortunately for us, it's not implemented the way we want in Windows. That doesn't mean there isn't another way around it, but I'm not confident there is, since the guys on the libusb mailing list have discussed the same problem in their Windows implementation. The "big hammer" approach would be to put all the threading into hidapi itself (and put all the reading on its own thread, filling a buffer, which would then be read from outside code (hid_read())). That's what I've had to do for mac and linux/libusb. Of course, then the read thread would have to have a way of being canceled and we're at the same problem... On the other platforms it's easier to do that part I guess.

mspisars commented 12 years ago

Is there a suggested work around for this on Windows? Can this still be reviewed for a possible solution? This breaks the attractiveness of the API being usable cross platform.

mspisars commented 12 years ago

Not a windows developer, but wonder if using GetQueuedCompletionStatus as mentioned in this SO post would be a viable solution to work around this limitation? http://stackoverflow.com/questions/961343/overlapped-i-o-how-to-wake-a-thread-on-a-completion-port-event-or-a-normal-even

signal11 commented 12 years ago

The functions you mention are also only available on Vista and newer. Unfortunately I don't have anything for Windows XP on this one.

You're relying on a close() function to cancel a blocking read(). This is typically undefined behavior on Unix systems, although it does work on non-Windows HIDAPI because I handle blocking myself (since it's all on userspace threads that I create).

You can modify your copy to use CancelIoEx() in hid_close(), or you could add support to look up that function dynamically(LoadLibrary()/GetProcAddress()), call it if available (and if not, call CancelIo()), and send me a patch. At least that would do what you want on Vista+.

Alternatively you could use hid_read_timeout() with a short (100ms, 200ms, 500ms?) timeout in your reading thread.

Keep in mind that on Windows, you can't call hid_read() from multiple threads concurrently without problems. All calls to hid_read() need to be on the same thread currently.

mspisars commented 7 years ago

So, is there any chance to have this feature implemented in standard hidapi? Win XP is on its 4th year past EOL. Would be a great help to remove these patches from our internal codebase.