[PATCH 0/1] MR9177: win32u: Fix deadlock caused by nested locking.
Convert surface lock to recursive mutex to allow same thread to acquire the lock multiple times, preventing deadlocks when lock_surface() and window_surface_lock() are called nestedly. Signed-off-by: Zhao Yi <zhaoyi(a)uniontech.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9177
From: Zhao Yi <zhaoyi(a)uniontech.com> Convert surface lock to recursive mutex to allow same thread to acquire the lock multiple times, preventing deadlocks when lock_surface() and window_surface_lock() are called nestedly. Signed-off-by: Zhao Yi <zhaoyi(a)uniontech.com> --- dlls/win32u/dce.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/win32u/dce.c b/dlls/win32u/dce.c index 6ba5f1c7c44..6c7e49b2976 100644 --- a/dlls/win32u/dce.c +++ b/dlls/win32u/dce.c @@ -528,6 +528,7 @@ W32KAPI struct window_surface *window_surface_create( UINT size, const struct wi const RECT *rect, BITMAPINFO *info, HBITMAP bitmap ) { struct window_surface *surface; + pthread_mutexattr_t attr; if (!(surface = calloc( 1, size ))) return NULL; surface->funcs = funcs; @@ -546,7 +547,10 @@ W32KAPI struct window_surface *window_surface_create( UINT size, const struct wi return NULL; } - pthread_mutex_init( &surface->mutex, NULL ); + pthread_mutexattr_init(&attr); + pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE_NP); + pthread_mutex_init( &surface->mutex, &attr ); + pthread_mutexattr_destroy(&attr); TRACE( "created surface %p for hwnd %p rect %s\n", surface, hwnd, wine_dbgstr_rect( &surface->rect ) ); return surface; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9177
Could you detail a bit more why it would need that? I think recursive locks are a sign that things are getting out of control and it would be better to remove the need for recursive locking instead. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9177#note_118548
On Wed Oct 15 06:26:43 2025 +0000, Rémi Bernon wrote:
Could you detail a bit more why it would need that? I think recursive locks are a sign that things are getting out of control and it would be better to remove the need for recursive locking instead. There is a scenario where the function call sequence in the same thread is: window_surface_lock() → lock_surface() → unlock_surface() → window_surface_unlock(). In this situation, window_surface_lock() is called first to acquire a lock. When lock_surface() is subsequently called to acquire the same lock, the initial lock has not yet been released, ultimately resulting in a deadlock.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9177#note_118549
On Wed Oct 15 06:26:43 2025 +0000, Zhao Yi wrote:
There is a scenario where the function call sequence in the same thread is: window_surface_lock() → lock_surface() → unlock_surface() → window_surface_unlock(). In this situation, window_surface_lock() is called first to acquire a lock. When lock_surface() is subsequently called to acquire the same lock, the initial lock has not yet been released, ultimately resulting in a deadlock. Sure, but what is this scenario exactly? I've worked to remove the surface recursive locking requirement before, and I think we should fix it that way instead.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9177#note_118550
On Wed Oct 15 06:30:38 2025 +0000, Rémi Bernon wrote:
Sure, but what is this scenario exactly? I've worked to remove the surface recursive locking requirement before, and I think we should fix it that way instead. This situation occurs when NtUserUpdateLayeredWindow is called. Within the NtUserUpdateLayeredWindow function, there exists the following call sequence: window_surface_lock() → NtGdiAlphaBlend() → window_surface_unlock(). However, the implementation of the NtGdiAlphaBlend() function calls the windrv_GetImage() function, which internally invokes lock_surface() and unlock_surface().
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9177#note_118551
On Wed Oct 15 06:36:25 2025 +0000, Zhao Yi wrote:
This situation occurs when NtUserUpdateLayeredWindow is called. Within the NtUserUpdateLayeredWindow function, there exists the following call sequence: window_surface_lock() → NtGdiAlphaBlend() → window_surface_unlock(). However, the implementation of the NtGdiAlphaBlend() function calls the windrv_GetImage() function, which internally invokes lock_surface() and unlock_surface(). I see, thank you. This happens when UpdateLayeredWindow is called with hdc_src set to one of the window's self DC right? We might be able to detect this case and handle it separately.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9177#note_118553
On Wed Oct 15 06:41:22 2025 +0000, Rémi Bernon wrote:
I see, thank you. This happens when UpdateLayeredWindow is called with hdc_src set to one of the window's self DC right? We might be able to detect this case and handle it separately. Yeah,That's right.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9177#note_118554
On Wed Oct 15 06:49:40 2025 +0000, Zhao Yi wrote:
Yeah,That's right. Would I need to close this MR and re-address this deadlock issue?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9177#note_118557
On Wed Oct 15 07:24:07 2025 +0000, Zhao Yi wrote:
Would I need to close this MR and re-address this deadlock issue? As you prefer, you can create a separate one with the alternative fix or update this one directly. I'm not completely settled about this being wrong so perhaps a separate MR would be better so we can decide depending on how complicated the other fix gets.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9177#note_118558
On Wed Oct 15 07:29:21 2025 +0000, Rémi Bernon wrote:
As you prefer, you can create a separate one with the alternative fix or update this one directly. I'm not completely settled about this being wrong so perhaps a separate MR would be better so we can decide depending on how complicated the other fix gets. OK ，I get it ,Thank you. I will close this MR and create a new one.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9177#note_118562
This merge request was closed by Zhao Yi. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9177
participants (3)
-
Rémi Bernon -
Zhao Yi
-
Zhao Yi (＠Zhaoyi)