encounter
/
SDL
mirror of https://github.com/encounter/SDL.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Do not wait in GetOverlappedResult() in hid_read_timeout() 

This is unsafe because the event is auto-reset, therefore the call to
WaitForSingleObject() resets the event which GetOverlappedResult() will
try to wait on.

Even though the overlapped operation is guaranteed to be completed at
the point we call GetOverlappedResult(), it will still wait on the event
handle for a short time to trigger the reset for auto-reset events. This
amounts to roughly a 100 ms sleep each time GetOverlappedResult() is called
for a completed I/O with a non-signalled event.

In the context of HIDAPI, this extra sleep means that callers that loop
on hid_read_timeout() with timeout=0 will loop forever, since the 100 ms
sleep each iteration ensures ReadFile() will always have new data.
This commit is contained in:
Cameron Gutman 2021-01-01 17:34:07 -06:00
parent 963c9495d3
commit 414ddc32c5
1 changed files with 11 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/hidapi/windows/hid.c
View File

@ -820,20 +820,19 @@ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char
}
}
if (milliseconds >= 0) {
/* See if there is any data yet. */
res = WaitForSingleObject(ev, milliseconds);
res = WaitForSingleObject(ev, milliseconds >= 0 ? milliseconds : INFINITE);
if (res != WAIT_OBJECT_0) {
/* There was no data this time. Return zero bytes available,
but leave the Overlapped I/O running. */
return 0;
}
}
/* Either WaitForSingleObject() told us that ReadFile has completed, or
we are in non-blocking mode. Get the number of bytes read. The actual
data has been copied to the data[] array which was passed to ReadFile(). */
res = GetOverlappedResult(dev->device_handle, &dev->ol, &bytes_read, TRUE/*wait*/);
/* Get the number of bytes read. The actual data has been copied to the data[]
array which was passed to ReadFile(). We must not wait here because we've
already waited on our event above, and since it's auto-reset, it will have
been reset back to unsignalled by now. */
res = GetOverlappedResult(dev->device_handle, &dev->ol, &bytes_read, FALSE/*don't wait*/);
/* Set pending back to false, even if GetOverlappedResult() returned error. */
dev->read_pending = FALSE;