user32: Leave critical section before calling callbacks in EnumDisplayMonitors().

Previously, callbacks were called with a critical section held. It was intended that monitor handles passed to callbacks should always be valid. But it created a deadlock condition when callbacks call other functions which try to grab the critical section using a different thread. Tests also show that a monitor handle can be invalid after a display change. So do not hold the critical section when calling callbacks. Monitor handles will be checked when passed to GetMonitorInfo(), which is the sole function that consumes HMONITORs. Signed-off-by: Zhiyi Zhang <zzhang@codeweavers.com> Signed-off-by: Alexandre Julliard <julliard@winehq.org>

user32: Leave critical section before calling callbacks in EnumDisplayMonitors().
449ca2fb · Zhiyi Zhang · Alexandre Julliard · 065690be · 449ca2fb
Commit 449ca2fb authored May 05, 2020 by Zhiyi Zhang Committed by Alexandre Julliard May 05, 2020
Hide whitespace changes
Inline Side-by-side

Showing with 15 additions and 9 deletions

sysparams.c dlls/user32/sysparams.c +15 -9

No files found.
--- a/dlls/user32/sysparams.c
+++ b/dlls/user32/sysparams.c
@@ -4126,28 +4126,34 @@ static BOOL CALLBACK enum_mon_callback( HMONITOR monitor, HDC hdc, LPRECT rect, 

 BOOL CDECL nulldrv_EnumDisplayMonitors( HDC hdc, RECT *rect, MONITORENUMPROC proc, LPARAM lp )
 {
-    RECT default_rect = {0, 0, 640, 480};
-    DWORD i;
+    RECT monitor_rect;
+    DWORD i = 0;

    TRACE("(%p, %p, %p, 0x%lx)\n", hdc, rect, proc, lp);

    if (update_monitor_cache())
    {
-        EnterCriticalSection( &monitors_section );
-        for (i = 0; i < monitor_count; i++)
+        while (TRUE)
        {
-            if (!proc( (HMONITOR)(UINT_PTR)(i + 1), hdc, &monitors[i].rcMonitor, lp ))
+            EnterCriticalSection( &monitors_section );
+            if (i >= monitor_count)
            {
                LeaveCriticalSection( &monitors_section );
-                return FALSE;
+                return TRUE;
            }
+            monitor_rect = monitors[i].rcMonitor;
+            LeaveCriticalSection( &monitors_section );
+
+            if (!proc( (HMONITOR)(UINT_PTR)(i + 1), hdc, &monitor_rect, lp ))
+                return FALSE;
+
+            ++i;
        }
-        LeaveCriticalSection( &monitors_section );
-        return TRUE;
    }

    /* Fallback to report one monitor if using SetupAPI failed */
-    if (!proc( NULLDRV_DEFAULT_HMONITOR, hdc, &default_rect, lp ))
+    SetRect( &monitor_rect, 0, 0, 640, 480 );
+    if (!proc( NULLDRV_DEFAULT_HMONITOR, hdc, &monitor_rect, lp ))
        return FALSE;
    return TRUE;
 }