[PATCH v8 0/1] MR2153: win32u: Invalidate all cached keys after input.
This patch addresses an issue in Second Life and potentially other multi-threaded applications which process WM_KEYDOWN in one thread and then verify that the key is "still down" with GetAsyncKeyState from another thread. Wine uses a per-thread key cache, resulting in inconsistent views of key status. Caches are now invalidated when an input event is injected by the driver or via SendInput. -- v8: win32u: Invalidate all cached keys after input. https://gitlab.winehq.org/wine/wine/-/merge_requests/2153
From: Henry Goffin <hgoffin(a)amazon.com> This patch addresses an issue in Second Life and potentially other multi-threaded applications which process WM_KEYDOWN in one thread and then verify that the key is "still down" with GetAsyncKeyState from another thread. Wine uses a per-thread key cache, resulting in inconsistent views of key status. Caches are now invalidated when an input event is injected by the driver or via SendInput. --- dlls/win32u/message.c | 18 ++++++++---------- include/wine/server_protocol.h | 1 - server/protocol.def | 1 - server/queue.c | 2 -- server/trace.c | 1 - 5 files changed, 8 insertions(+), 15 deletions(-) diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index de1ca8cefbc..5ca568d7302 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -2585,13 +2585,11 @@ LRESULT send_internal_message_timeout( DWORD dest_pid, DWORD dest_tid, */ NTSTATUS send_hardware_message( HWND hwnd, const INPUT *input, const RAWINPUT *rawinput, UINT flags ) { - struct user_key_state_info *key_state_info = get_user_thread_info()->key_state; struct send_message_info info; int prev_x, prev_y, new_x, new_y; - INT counter = global_key_state_counter; USAGE hid_usage_page, hid_usage; NTSTATUS ret; - BOOL wait; + BOOL wait, affects_key_state = FALSE; info.type = MSG_HARDWARE; info.dest_tid = 0; @@ -2629,6 +2627,10 @@ NTSTATUS send_hardware_message( HWND hwnd, const INPUT *input, const RAWINPUT *r req->input.mouse.flags = input->mi.dwFlags; req->input.mouse.time = input->mi.time; req->input.mouse.info = input->mi.dwExtraInfo; + affects_key_state = !!(input->mi.dwFlags & (MOUSEEVENTF_LEFTDOWN | MOUSEEVENTF_LEFTUP | + MOUSEEVENTF_RIGHTDOWN | MOUSEEVENTF_RIGHTUP | + MOUSEEVENTF_MIDDLEDOWN | MOUSEEVENTF_MIDDLEUP | + MOUSEEVENTF_XDOWN | MOUSEEVENTF_XUP)); break; case INPUT_KEYBOARD: req->input.kbd.vkey = input->ki.wVk; @@ -2636,6 +2638,7 @@ NTSTATUS send_hardware_message( HWND hwnd, const INPUT *input, const RAWINPUT *r req->input.kbd.flags = input->ki.dwFlags; req->input.kbd.time = input->ki.time; req->input.kbd.info = input->ki.dwExtraInfo; + affects_key_state = TRUE; break; case INPUT_HARDWARE: req->input.hw.msg = input->hi.uMsg; @@ -2664,8 +2667,6 @@ NTSTATUS send_hardware_message( HWND hwnd, const INPUT *input, const RAWINPUT *r } break; } - if (key_state_info) wine_server_set_reply( req, key_state_info->state, - sizeof(key_state_info->state) ); ret = wine_server_call( req ); wait = reply->wait; prev_x = reply->prev_x; @@ -2677,11 +2678,8 @@ NTSTATUS send_hardware_message( HWND hwnd, const INPUT *input, const RAWINPUT *r if (!ret) { - if (key_state_info) - { - key_state_info->time = NtGetTickCount(); - key_state_info->counter = counter; - } + if (affects_key_state) + InterlockedIncrement( &global_key_state_counter ); /* force refreshing the key state cache */ if ((flags & SEND_HWMSG_INJECTED) && (prev_x != new_x || prev_y != new_y)) user_driver->pSetCursorPos( new_x, new_y ); } diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index c5e0182f42a..9dfe936057b 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -2768,7 +2768,6 @@ struct send_hardware_message_reply int prev_y; int new_x; int new_y; - /* VARARG(keystate,bytes); */ char __pad_28[4]; }; #define SEND_HWMSG_INJECTED 0x01 diff --git a/server/protocol.def b/server/protocol.def index 69888fa643e..57957efdae3 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -2078,7 +2078,6 @@ enum message_type int prev_y; int new_x; /* new cursor position */ int new_y; - VARARG(keystate,bytes); /* global state array for all the keys */ @END #define SEND_HWMSG_INJECTED 0x01 diff --git a/server/queue.c b/server/queue.c index 07a31b47370..7320dddc6b7 100644 --- a/server/queue.c +++ b/server/queue.c @@ -2551,7 +2551,6 @@ DECL_HANDLER(send_hardware_message) struct desktop *desktop; unsigned int origin = (req->flags & SEND_HWMSG_INJECTED ? IMO_INJECTED : IMO_HARDWARE); struct msg_queue *sender = get_current_queue(); - data_size_t size = min( 256, get_reply_max_size() ); if (!(desktop = get_thread_desktop( current, 0 ))) return; @@ -2587,7 +2586,6 @@ DECL_HANDLER(send_hardware_message) reply->new_x = desktop->cursor.x; reply->new_y = desktop->cursor.y; - set_reply_data( desktop->keystate, size ); release_object( desktop ); } diff --git a/server/trace.c b/server/trace.c index b63bc386ee8..91bc3ae7af9 100644 --- a/server/trace.c +++ b/server/trace.c @@ -2662,7 +2662,6 @@ static void dump_send_hardware_message_reply( const struct send_hardware_message fprintf( stderr, ", prev_y=%d", req->prev_y ); fprintf( stderr, ", new_x=%d", req->new_x ); fprintf( stderr, ", new_y=%d", req->new_y ); - dump_varargs_bytes( ", keystate=", cur_size ); } static void dump_get_message_request( const struct get_message_request *req ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2153
On Wed Mar 1 20:43:56 2023 +0000, Zebediah Figura wrote:
This is missing the change to protocol.def. So it is. Thanks for catching it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2153#note_25900
participants (2)
-
Henry Goffin -
Henry Goffin (＠AMZN-hgoffin)