hidapi/windows: do not wait in GetOverlappedResult() in hid_read_timeout() (#577)

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.

Signed-off-by: Cameron Gutman <[email protected]>
Signed-off-by: Sam Lantinga <[email protected]>

Author: slouken

windows/hid.c

@@ -1163,20 +1163,19 @@ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char
 	}
 
 	if (overlapped) {
-		if (milliseconds >= 0) {
-			/* See if there is any data yet. */
-			res = WaitForSingleObject(ev, milliseconds);
-			if (res != WAIT_OBJECT_0) {
-				/* There was no data this time. Return zero bytes available,
-				   but leave the Overlapped I/O running. */
-				return 0;
-			}
+		/* See if there is any data yet. */
+		res = WaitForSingleObject(ev, milliseconds >= 0 ? (DWORD)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 now - already did on the prev step*/);
 	}
 	/* Set pending back to false, even if GetOverlappedResult() returned error. */
 	dev->read_pending = FALSE;