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@uniontech.com
From: Zhao Yi zhaoyi@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@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;
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.
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.
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.
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().
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.
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.
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?
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.
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.
This merge request was closed by Zhao Yi.
-
Rémi Bernon (＠rbernon) -
Zhao Yi
-
Zhao Yi (＠Zhaoyi)