[PATCH 0/1] MR9544: win32u: Release internal OpenGL drawables outside of the user lock.
Sometimes windows get destroyed while wined3d still has some internal drawables allocated, and the cached DCs gets removed and destroyed as soon as the window itself is, causing free_dce to be called with the user lock held already. Then opengl_drawable_release grabing the surfaces lock with the user lock held causes deadlocks with other threads creating or presenting surfaces which need to take the locks in the reverse order. Fixes: a085746bbe3594e4ea01ea578823383e6447b6b6 This is visible in several d2d/d3d tests timing out since that change. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9544
From: Rémi Bernon <rbernon(a)codeweavers.com> Sometimes windows get destroyed while wined3d still has some internal drawables allocated, and the cached DCs gets removed and destroyed as soon as the window itself is, causing free_dce to be called with the user lock held already. Then opengl_drawable_release grabing the surfaces lock with the user lock held causes deadlocks with other threads creating or presenting surfaces which need to take the locks in the reverse order. Fixes: a085746bbe3594e4ea01ea578823383e6447b6b6 --- dlls/win32u/class.c | 5 ++++- dlls/win32u/dce.c | 13 +++++-------- dlls/win32u/ntuser_private.h | 5 +++-- dlls/win32u/opengl.c | 20 +++++++++++++++++--- dlls/win32u/window.c | 11 ++++++++--- include/wine/opengl_driver.h | 1 + 6 files changed, 38 insertions(+), 17 deletions(-) diff --git a/dlls/win32u/class.c b/dlls/win32u/class.c index ba1f7d650af..9cc5b6402aa 100644 --- a/dlls/win32u/class.c +++ b/dlls/win32u/class.c @@ -615,6 +615,7 @@ ATOM WINAPI NtUserRegisterClassExWOW( const WNDCLASSEXW *wc, UNICODE_STRING *nam BOOL WINAPI NtUserUnregisterClass( UNICODE_STRING *name, HINSTANCE instance, struct client_menu_name *client_menu_name ) { + struct list drawables = LIST_INIT( drawables ); CLASS *class = NULL; /* create the desktop window to trigger builtin class registration */ @@ -632,7 +633,7 @@ BOOL WINAPI NtUserUnregisterClass( UNICODE_STRING *name, HINSTANCE instance, TRACE( "%p\n", class ); user_lock(); - if (class->dce) free_dce( class->dce, 0 ); + if (class->dce) free_dce( class->dce, 0, &drawables ); list_remove( &class->entry ); if (class->hbrBackground > (HBRUSH)(COLOR_GRADIENTINACTIVECAPTION + 1)) NtGdiDeleteObjectApp( class->hbrBackground ); @@ -640,6 +641,8 @@ BOOL WINAPI NtUserUnregisterClass( UNICODE_STRING *name, HINSTANCE instance, NtUserDestroyCursor( class->hIconSmIntern, 0 ); free( class ); user_unlock(); + + release_opengl_drawables( &drawables ); return TRUE; } diff --git a/dlls/win32u/dce.c b/dlls/win32u/dce.c index c84ba693509..d720d686bcd 100644 --- a/dlls/win32u/dce.c +++ b/dlls/win32u/dce.c @@ -1068,9 +1068,8 @@ static struct dce *get_window_dce( HWND hwnd ) * * Free a class or window DCE. */ -void free_dce( struct dce *dce, HWND hwnd ) +void free_dce( struct dce *dce, HWND hwnd, struct list *drawables ) { - struct opengl_drawable *drawable = NULL; struct dce *dce_to_free = NULL; user_lock(); @@ -1103,7 +1102,7 @@ void free_dce( struct dce *dce, HWND hwnd ) { WARN( "GetDC() without ReleaseDC() for window %p\n", hwnd ); dce->count = 0; - set_dc_pixel_format_internal( dce->hdc, 0, &drawable ); + set_dc_pixel_format_internal( dce->hdc, 0, drawables ); set_dce_flags( dce->hdc, DCHF_DISABLEDC ); } } @@ -1117,8 +1116,6 @@ void free_dce( struct dce *dce, HWND hwnd ) NtGdiDeleteObjectApp( dce_to_free->hdc ); free( dce_to_free ); } - - if (drawable) opengl_drawable_release( drawable ); } BOOL is_cache_dc( HDC hdc ) @@ -1226,7 +1223,7 @@ void invalidate_dce( WND *win, const RECT *old_rect ) */ static INT release_dc( HWND hwnd, HDC hdc, BOOL end_paint ) { - struct opengl_drawable *drawable = NULL; + struct list drawables = LIST_INIT( drawables ); struct dce *dce; BOOL ret = FALSE; @@ -1241,14 +1238,14 @@ static INT release_dc( HWND hwnd, HDC hdc, BOOL end_paint ) if (dce->flags & DCX_CACHE) { dce->count = 0; - set_dc_pixel_format_internal( hdc, 0, &drawable ); + set_dc_pixel_format_internal( hdc, 0, &drawables ); set_dce_flags( dce->hdc, DCHF_DISABLEDC ); } ret = TRUE; } user_unlock(); - if (drawable) opengl_drawable_release( drawable ); + release_opengl_drawables( &drawables ); return ret; } diff --git a/dlls/win32u/ntuser_private.h b/dlls/win32u/ntuser_private.h index c38b6d0e029..14dcff3ad38 100644 --- a/dlls/win32u/ntuser_private.h +++ b/dlls/win32u/ntuser_private.h @@ -208,7 +208,7 @@ extern LRESULT drag_drop_call( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam HICON alloc_cursoricon_handle( BOOL is_icon ); /* dce.c */ -extern void free_dce( struct dce *dce, HWND hwnd ); +extern void free_dce( struct dce *dce, HWND hwnd, struct list *drawables ); extern void invalidate_dce( WND *win, const RECT *old_rect ); extern BOOL is_cache_dc( HDC hdc ); @@ -219,7 +219,8 @@ extern void check_for_events( UINT flags ); extern LRESULT system_tray_call( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam, void *data ); /* opengl.c */ -extern BOOL set_dc_pixel_format_internal( HDC hdc, int format, struct opengl_drawable **drawable ); +extern BOOL set_dc_pixel_format_internal( HDC hdc, int format, struct list *drawables ); +extern void release_opengl_drawables( struct list *drawables ); /* vulkan.c */ extern struct vulkan_instance *vulkan_instance_create( const struct vulkan_instance_extensions *extensions ); diff --git a/dlls/win32u/opengl.c b/dlls/win32u/opengl.c index c4aed62a3ae..f231eacda83 100644 --- a/dlls/win32u/opengl.c +++ b/dlls/win32u/opengl.c @@ -1511,21 +1511,33 @@ static BOOL win32u_wglSetPixelFormat( HDC hdc, int new_format, const PIXELFORMAT return NtGdiSetPixelFormat( hdc, new_format ); } -BOOL set_dc_pixel_format_internal( HDC hdc, int format, struct opengl_drawable **drawable ) +BOOL set_dc_pixel_format_internal( HDC hdc, int format, struct list *drawables ) { DC *dc; if (!(dc = get_dc_ptr( hdc ))) return FALSE; dc->pixel_format = format; - *drawable = dc->opengl_drawable; + if (dc->opengl_drawable) list_add_tail( drawables, &dc->opengl_drawable->entry ); dc->opengl_drawable = NULL; release_dc_ptr( dc ); return TRUE; } +void release_opengl_drawables( struct list *drawables ) +{ + struct opengl_drawable *drawable, *next; + + LIST_FOR_EACH_ENTRY_SAFE( drawable, next, drawables, struct opengl_drawable, entry ) + { + list_remove( &drawable->entry ); + opengl_drawable_release( drawable ); + } +} + static BOOL win32u_wglSetPixelFormatWINE( HDC hdc, int format ) { + struct list *ptr, drawables = LIST_INIT( drawables ); const struct opengl_funcs *funcs = &display_funcs; struct opengl_drawable *drawable; UINT total, onscreen; @@ -1540,7 +1552,9 @@ static BOOL win32u_wglSetPixelFormatWINE( HDC hdc, int format ) if (format < 0 || format > total) return FALSE; if (format > onscreen) return FALSE; - if (!set_dc_pixel_format_internal( hdc, format, &drawable )) return FALSE; + if (!set_dc_pixel_format_internal( hdc, format, &drawables )) return FALSE; + if (!(ptr = list_head( &drawables ))) drawable = NULL; + else drawable = LIST_ENTRY( ptr, struct opengl_drawable, entry ); if (drawable && drawable->format != format) { diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index 00023632d13..55c89716a23 100644 --- a/dlls/win32u/window.c +++ b/dlls/win32u/window.c @@ -5216,7 +5216,7 @@ static void free_window_handle( HWND hwnd ) */ LRESULT destroy_window( HWND hwnd ) { - struct list vulkan_surfaces = LIST_INIT(vulkan_surfaces); + struct list drawables = LIST_INIT( drawables ); struct window_surface *surface; HMENU menu = 0, sys_menu; WND *win; @@ -5262,13 +5262,15 @@ LRESULT destroy_window( HWND hwnd ) if ((win->dwStyle & (WS_CHILD | WS_POPUP)) != WS_CHILD) menu = (HMENU)win->wIDmenu; sys_menu = win->hSysMenu; - free_dce( win->dce, hwnd ); + free_dce( win->dce, hwnd, &drawables ); win->dce = NULL; NtUserDestroyCursor( win->hIconSmall2, 0 ); surface = win->surface; win->surface = NULL; release_win_ptr( win ); + release_opengl_drawables( &drawables ); + NtUserDestroyMenu( menu ); NtUserDestroyMenu( sys_menu ); if (surface) @@ -5389,6 +5391,7 @@ void destroy_thread_windows(void) struct window_surface *surface; struct destroy_entry *next; } *entry, *free_list = NULL; + struct list drawables = LIST_INIT(drawables); HANDLE handle = 0; WND *win; @@ -5401,7 +5404,7 @@ void destroy_thread_windows(void) BOOL is_child = (win->dwStyle & (WS_CHILD | WS_POPUP)) == WS_CHILD; struct destroy_entry tmp = {0}; - free_dce( win->dce, win->handle ); + free_dce( win->dce, win->handle, &drawables ); set_user_handle_ptr( handle, NULL ); free( win->pScroll ); free( win->text ); @@ -5430,6 +5433,8 @@ void destroy_thread_windows(void) } user_unlock(); + release_opengl_drawables( &drawables ); + while ((entry = free_list)) { free_list = entry->next; diff --git a/include/wine/opengl_driver.h b/include/wine/opengl_driver.h index e9052392471..30a9537a714 100644 --- a/include/wine/opengl_driver.h +++ b/include/wine/opengl_driver.h @@ -176,6 +176,7 @@ struct opengl_drawable { const struct opengl_drawable_funcs *funcs; LONG ref; /* reference count */ + struct list entry; /* entry in cleanup lists */ struct client_surface *client; /* underlying client surface */ int format; /* pixel format of the drawable */ int interval; /* last set surface swap interval */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9544
participants (1)
-
Rémi Bernon