Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hid_close does not abort hid_read #48

Open
vanweric opened this issue Mar 27, 2012 · 8 comments
Open

hid_close does not abort hid_read #48

vanweric opened this issue Mar 27, 2012 · 8 comments

Comments

@vanweric
Copy link

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Contributor

mspisars commented Jun 2, 2012

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
Copy link
Contributor

mspisars commented Jun 3, 2012

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
Copy link
Owner

signal11 commented Jun 3, 2012

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
Copy link
Contributor

mspisars commented Oct 25, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants