[PATCH] winex11.drv: Avoid re-creating the clipping message window if it already exists
When the same thread repeatedly calls ClipCursor, a message window was created for every call, then stored in x11drv_thread_data->clip_hwnd. The WM_X11DRV_CLIP_CURSOR notification is then sent to the desktop window, targetting the previous clipping thread in order for it to destroy its clip_hwnd. But as x11drv_thread_data->clip_hwnd has been overwritten, it does not satisfy the "hwnd == data->clip_hwnd" condition and the previous clip_hwnd is leaked. This leads to exhaustion of user handles, and ultimately, failures to create new clipping message windows, which then in turn makes the pointer grab be lost. --- dlls/winex11.drv/mouse.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/dlls/winex11.drv/mouse.c b/dlls/winex11.drv/mouse.c index f737a306a56..3ab6bf5a1ec 100644 --- a/dlls/winex11.drv/mouse.c +++ b/dlls/winex11.drv/mouse.c @@ -388,7 +388,8 @@ static BOOL grab_clipping_window( const RECT *clip ) if (!data) return FALSE; if (!(clip_window = init_clip_window())) return TRUE; - if (!(msg_hwnd = CreateWindowW( messageW, NULL, 0, 0, 0, 0, 0, HWND_MESSAGE, 0, + if (!data->clip_hwnd && + !(msg_hwnd = CreateWindowW( messageW, NULL, 0, 0, 0, 0, 0, HWND_MESSAGE, 0, GetModuleHandleW(0), NULL ))) return TRUE; @@ -398,12 +399,12 @@ static BOOL grab_clipping_window( const RECT *clip ) if (data->xi2_state != xi_enabled) { WARN( "XInput2 not supported, refusing to clip to %s\n", wine_dbgstr_rect(clip) ); - DestroyWindow( msg_hwnd ); + if (msg_hwnd) DestroyWindow( msg_hwnd ); ClipCursor( NULL ); return TRUE; } - TRACE( "clipping to %s win %lx\n", wine_dbgstr_rect(clip), clip_window ); + TRACE( "clipping to %s win %p/%lx\n", wine_dbgstr_rect(clip), data->clip_hwnd ? data->clip_hwnd : msg_hwnd, clip_window ); if (!data->clip_hwnd) XUnmapWindow( data->display, clip_window ); pos = virtual_screen_to_root( clip->left, clip->top ); @@ -424,14 +425,19 @@ static BOOL grab_clipping_window( const RECT *clip ) if (!clipping_cursor) { disable_xinput2(); - DestroyWindow( msg_hwnd ); + if (msg_hwnd) DestroyWindow( msg_hwnd ); return FALSE; } clip_rect = *clip; - if (!data->clip_hwnd) sync_window_cursor( clip_window ); - InterlockedExchangePointer( (void **)&cursor_window, msg_hwnd ); - data->clip_hwnd = msg_hwnd; - SendMessageW( GetDesktopWindow(), WM_X11DRV_CLIP_CURSOR, 0, (LPARAM)msg_hwnd ); + + if (!data->clip_hwnd) + { + sync_window_cursor( clip_window ); + data->clip_hwnd = msg_hwnd; + } + + InterlockedExchangePointer( (void **)&cursor_window, data->clip_hwnd ); + SendMessageW( GetDesktopWindow(), WM_X11DRV_CLIP_CURSOR, 0, (LPARAM)data->clip_hwnd ); return TRUE; } @@ -481,6 +487,7 @@ LRESULT clip_cursor_notify( HWND hwnd, HWND new_clip_hwnd ) HWND prev = clip_hwnd; clip_hwnd = new_clip_hwnd; + if (prev == new_clip_hwnd) return 0; if (prev || new_clip_hwnd) TRACE( "clip hwnd changed from %p to %p\n", prev, new_clip_hwnd ); if (prev) SendNotifyMessageW( prev, WM_X11DRV_CLIP_CURSOR, 0, 0 ); } -- 2.20.1
When the same thread repeatedly calls ClipCursor, a message window was created for every call, then stored in x11drv_thread_data->clip_hwnd. The WM_X11DRV_CLIP_CURSOR notification is then sent to the desktop window, targetting the previous clipping thread in order for it to destroy its clip_hwnd. But as x11drv_thread_data->clip_hwnd has been overwritten, it does not satisfy the "hwnd == data->clip_hwnd" condition and the previous clip_hwnd is leaked. This leads to exhaustion of user handles, and ultimately, failures to create new clipping message windows, which then in turn makes the pointer grab be lost. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- Resending with Signed-off-by. dlls/winex11.drv/mouse.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/dlls/winex11.drv/mouse.c b/dlls/winex11.drv/mouse.c index f737a306a56..3ab6bf5a1ec 100644 --- a/dlls/winex11.drv/mouse.c +++ b/dlls/winex11.drv/mouse.c @@ -388,7 +388,8 @@ static BOOL grab_clipping_window( const RECT *clip ) if (!data) return FALSE; if (!(clip_window = init_clip_window())) return TRUE; - if (!(msg_hwnd = CreateWindowW( messageW, NULL, 0, 0, 0, 0, 0, HWND_MESSAGE, 0, + if (!data->clip_hwnd && + !(msg_hwnd = CreateWindowW( messageW, NULL, 0, 0, 0, 0, 0, HWND_MESSAGE, 0, GetModuleHandleW(0), NULL ))) return TRUE; @@ -398,12 +399,12 @@ static BOOL grab_clipping_window( const RECT *clip ) if (data->xi2_state != xi_enabled) { WARN( "XInput2 not supported, refusing to clip to %s\n", wine_dbgstr_rect(clip) ); - DestroyWindow( msg_hwnd ); + if (msg_hwnd) DestroyWindow( msg_hwnd ); ClipCursor( NULL ); return TRUE; } - TRACE( "clipping to %s win %lx\n", wine_dbgstr_rect(clip), clip_window ); + TRACE( "clipping to %s win %p/%lx\n", wine_dbgstr_rect(clip), data->clip_hwnd ? data->clip_hwnd : msg_hwnd, clip_window ); if (!data->clip_hwnd) XUnmapWindow( data->display, clip_window ); pos = virtual_screen_to_root( clip->left, clip->top ); @@ -424,14 +425,19 @@ static BOOL grab_clipping_window( const RECT *clip ) if (!clipping_cursor) { disable_xinput2(); - DestroyWindow( msg_hwnd ); + if (msg_hwnd) DestroyWindow( msg_hwnd ); return FALSE; } clip_rect = *clip; - if (!data->clip_hwnd) sync_window_cursor( clip_window ); - InterlockedExchangePointer( (void **)&cursor_window, msg_hwnd ); - data->clip_hwnd = msg_hwnd; - SendMessageW( GetDesktopWindow(), WM_X11DRV_CLIP_CURSOR, 0, (LPARAM)msg_hwnd ); + + if (!data->clip_hwnd) + { + sync_window_cursor( clip_window ); + data->clip_hwnd = msg_hwnd; + } + + InterlockedExchangePointer( (void **)&cursor_window, data->clip_hwnd ); + SendMessageW( GetDesktopWindow(), WM_X11DRV_CLIP_CURSOR, 0, (LPARAM)data->clip_hwnd ); return TRUE; } @@ -481,6 +487,7 @@ LRESULT clip_cursor_notify( HWND hwnd, HWND new_clip_hwnd ) HWND prev = clip_hwnd; clip_hwnd = new_clip_hwnd; + if (prev == new_clip_hwnd) return 0; if (prev || new_clip_hwnd) TRACE( "clip hwnd changed from %p to %p\n", prev, new_clip_hwnd ); if (prev) SendNotifyMessageW( prev, WM_X11DRV_CLIP_CURSOR, 0, 0 ); } -- 2.20.1
participants (1)
-
Rémi Bernon