Module: wine Branch: master Commit: 606dc246ff0e6ac710288e4d9dc2fcf55568b1af URL: https://source.winehq.org/git/wine.git/?a=commit;h=606dc246ff0e6ac710288e4d9...
Author: Rémi Bernon rbernon@codeweavers.com Date: Fri May 6 15:10:03 2022 +0200
dinput/tests: Wait for the expected report to actually be pending.
There is a race otherwise where we try to complete a pending IRP but because the async is writing the report from another thread we didn't find it and instead ignored it.
Instead we need to atomically check if there was a pending IRP, and if the queue is empty, or queue the wait.
Later, when a report is going to be marked as pending, and if there's someone waiting for it already, we instead complete it immediately.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com Signed-off-by: Alexandre Julliard julliard@winehq.org
---
dlls/dinput/tests/dinput_test.h | 5 +++-- dlls/dinput/tests/driver_bus.c | 45 ++++++++++++++++++++++++++++---------- dlls/dinput/tests/driver_hid.h | 7 +++++- dlls/dinput/tests/force_feedback.c | 4 ++-- dlls/dinput/tests/hid.c | 6 +++-- 5 files changed, 48 insertions(+), 19 deletions(-)
diff --git a/dlls/dinput/tests/dinput_test.h b/dlls/dinput/tests/dinput_test.h index 0adbd0b1ac8..c4ee42ca1c8 100644 --- a/dlls/dinput/tests/dinput_test.h +++ b/dlls/dinput/tests/dinput_test.h @@ -103,8 +103,9 @@ BOOL sync_ioctl_( const char *file, int line, HANDLE device, DWORD code, void *i #define set_hid_expect( a, b, c ) set_hid_expect_( __FILE__, __LINE__, a, b, c ) void set_hid_expect_( const char *file, int line, HANDLE device, struct hid_expect *expect, DWORD expect_size );
-#define wait_hid_expect( a, b ) wait_hid_expect_( __FILE__, __LINE__, a, b, FALSE ) -void wait_hid_expect_( const char *file, int line, HANDLE device, DWORD timeout, BOOL todo ); +#define wait_hid_expect( a, b ) wait_hid_expect_( __FILE__, __LINE__, a, b, FALSE, FALSE ) +#define wait_hid_pending( a, b ) wait_hid_expect_( __FILE__, __LINE__, a, b, TRUE, FALSE ) +void wait_hid_expect_( const char *file, int line, HANDLE device, DWORD timeout, BOOL wait_pending, BOOL todo );
#define send_hid_input( a, b, c ) send_hid_input_( __FILE__, __LINE__, a, b, c ) void send_hid_input_( const char *file, int line, HANDLE device, struct hid_expect *expect, DWORD expect_size ); diff --git a/dlls/dinput/tests/driver_bus.c b/dlls/dinput/tests/driver_bus.c index 6554d50c14b..6a1994e509c 100644 --- a/dlls/dinput/tests/driver_bus.c +++ b/dlls/dinput/tests/driver_bus.c @@ -188,26 +188,34 @@ static NTSTATUS expect_queue_add_pending( struct expect_queue *queue, IRP *irp ) return status; }
-static void expect_queue_clear_pending( struct expect_queue *queue ) +/* complete an expect report previously marked as pending, or wait for one and then for the queue to empty */ +static NTSTATUS expect_queue_wait_pending( struct expect_queue *queue, IRP *irp ) { + NTSTATUS status; + IRP *pending; KIRQL irql; - IRP *irp;
KeAcquireSpinLock( &queue->lock, &irql ); - if ((irp = queue->pending_wait)) + if ((pending = queue->pending_wait)) { queue->pending_wait = NULL; - if (!IoSetCancelRoutine( irp, NULL )) irp = NULL; + if (!IoSetCancelRoutine( pending, NULL )) pending = NULL; } + + if (pending && queue->pos == queue->end) status = STATUS_SUCCESS; + else status = expect_queue_add_pending_locked( queue, irp ); KeReleaseSpinLock( &queue->lock, irql );
- if (irp) + if (pending) { - irp->IoStatus.Status = STATUS_SUCCESS; - IoCompleteRequest( irp, IO_NO_INCREMENT ); + pending->IoStatus.Status = STATUS_SUCCESS; + IoCompleteRequest( pending, IO_NO_INCREMENT ); } + + return status; }
+/* wait for the expect queue to empty */ static NTSTATUS expect_queue_wait( struct expect_queue *queue, IRP *irp ) { NTSTATUS status; @@ -257,11 +265,21 @@ static void expect_queue_next( struct expect_queue *queue, ULONG code, HID_XFER_ if (running_under_wine || !queue->pos->wine_only) break; queue->pos++; } - if (queue->pos == queue->end && (irp = queue->pending_wait)) + + if ((irp = queue->pending_wait)) { - queue->pending_wait = NULL; - if (!IoSetCancelRoutine( irp, NULL )) irp = NULL; + /* don't mark the IRP as pending if someone's already waiting */ + if (expect->ret_status == STATUS_PENDING) expect->ret_status = STATUS_SUCCESS; + + /* complete the pending wait IRP if the queue is now empty */ + if (queue->pos != queue->end) irp = NULL; + else + { + queue->pending_wait = NULL; + if (!IoSetCancelRoutine( irp, NULL )) irp = NULL; + } } + memcpy( context, queue->context, context_size ); KeReleaseSpinLock( &queue->lock, irql );
@@ -1244,9 +1262,12 @@ static NTSTATUS WINAPI pdo_ioctl( DEVICE_OBJECT *device, IRP *irp ) status = STATUS_SUCCESS; break; case IOCTL_WINETEST_HID_WAIT_EXPECT: - expect_queue_clear_pending( &impl->expect_queue ); - status = expect_queue_wait( &impl->expect_queue, irp ); + { + struct wait_expect_params wait_params = *(struct wait_expect_params *)irp->AssociatedIrp.SystemBuffer; + if (!wait_params.wait_pending) status = expect_queue_wait( &impl->expect_queue, irp ); + else status = expect_queue_wait_pending( &impl->expect_queue, irp ); break; + } case IOCTL_WINETEST_HID_SEND_INPUT: input_queue_reset( &impl->input_queue, irp->AssociatedIrp.SystemBuffer, in_size ); status = STATUS_SUCCESS; diff --git a/dlls/dinput/tests/driver_hid.h b/dlls/dinput/tests/driver_hid.h index 4711d391bfe..c5641097df3 100644 --- a/dlls/dinput/tests/driver_hid.h +++ b/dlls/dinput/tests/driver_hid.h @@ -42,7 +42,7 @@ DEFINE_GUID(control_class,0xdeadbeef,0x29ef,0x4538,0xa5,0xfd,0xb6,0x95,0x73,0xa3,0x62,0xc0);
#define IOCTL_WINETEST_HID_SET_EXPECT CTL_CODE(FILE_DEVICE_KEYBOARD, 0x800, METHOD_IN_DIRECT, FILE_ANY_ACCESS) -#define IOCTL_WINETEST_HID_WAIT_EXPECT CTL_CODE(FILE_DEVICE_KEYBOARD, 0x801, METHOD_NEITHER, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_HID_WAIT_EXPECT CTL_CODE(FILE_DEVICE_KEYBOARD, 0x801, METHOD_IN_DIRECT, FILE_ANY_ACCESS) #define IOCTL_WINETEST_HID_SEND_INPUT CTL_CODE(FILE_DEVICE_KEYBOARD, 0x802, METHOD_IN_DIRECT, FILE_ANY_ACCESS) #define IOCTL_WINETEST_HID_SET_CONTEXT CTL_CODE(FILE_DEVICE_KEYBOARD, 0x803, METHOD_IN_DIRECT, FILE_ANY_ACCESS) #define IOCTL_WINETEST_CREATE_DEVICE CTL_CODE(FILE_DEVICE_KEYBOARD, 0x804, METHOD_IN_DIRECT, FILE_ANY_ACCESS) @@ -61,6 +61,11 @@ struct hid_expect BYTE report_buf[128]; };
+struct wait_expect_params +{ + BOOL wait_pending; +}; + /* create/remove device */ #define MAX_HID_DESCRIPTOR_LEN 2048
diff --git a/dlls/dinput/tests/force_feedback.c b/dlls/dinput/tests/force_feedback.c index f5b387f3566..976b1b93863 100644 --- a/dlls/dinput/tests/force_feedback.c +++ b/dlls/dinput/tests/force_feedback.c @@ -5536,7 +5536,7 @@ static void test_windows_gaming_input(void) ok( !bool_async_handler.invoked, "handler invoked\n" ); IAsyncInfo_Release( async_info );
- wait_hid_expect( file, 100 ); + wait_hid_pending( file, 100 ); ret = WaitForSingleObject( bool_async_handler.event, 100 ); ok( ret == 0, "WaitForSingleObject returned %#lx\n", ret ); CloseHandle( bool_async_handler.event ); @@ -5581,7 +5581,7 @@ static void test_windows_gaming_input(void) ok( !bool_async_handler.invoked, "handler invoked\n" ); IAsyncInfo_Release( async_info );
- wait_hid_expect( file, 500 ); + wait_hid_pending( file, 100 ); ret = WaitForSingleObject( bool_async_handler.event, 100 ); ok( ret == 0, "WaitForSingleObject returned %#lx\n", ret ); CloseHandle( bool_async_handler.event ); diff --git a/dlls/dinput/tests/hid.c b/dlls/dinput/tests/hid.c index 7e2d9cf86cf..00a3d65fe4d 100644 --- a/dlls/dinput/tests/hid.c +++ b/dlls/dinput/tests/hid.c @@ -911,10 +911,12 @@ void set_hid_expect_( const char *file, int line, HANDLE device, struct hid_expe ok_(file, line)( ret, "IOCTL_WINETEST_HID_SET_EXPECT failed, last error %lu\n", GetLastError() ); }
-void wait_hid_expect_( const char *file, int line, HANDLE device, DWORD timeout, BOOL todo ) +void wait_hid_expect_( const char *file, int line, HANDLE device, DWORD timeout, BOOL wait_pending, BOOL todo ) { + struct wait_expect_params params = {.wait_pending = wait_pending}; + todo_wine_if(todo) { - BOOL ret = sync_ioctl_( file, line, device, IOCTL_WINETEST_HID_WAIT_EXPECT, NULL, 0, NULL, 0, timeout ); + BOOL ret = sync_ioctl_( file, line, device, IOCTL_WINETEST_HID_WAIT_EXPECT, ¶ms, sizeof(params), NULL, 0, timeout ); ok_(file, line)( ret, "IOCTL_WINETEST_HID_WAIT_EXPECT failed, last error %lu\n", GetLastError() ); }