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

[Alexandre Julliard]

Module: wine
Branch: master
Commit: 449ca2fbf9e44a40ff2cc6eedada8dd2f94c7adf
URL:    https://source.winehq.org/git/wine.git/?a=commit;h=449ca2fbf9e44a40ff2cc6eed...

Author: Zhiyi Zhang <zzhang(a)codeweavers.com>
Date:   Tue May  5 20:30:39 2020 +0800

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(a)codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard(a)winehq.org>

---

 dlls/user32/sysparams.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/dlls/user32/sysparams.c b/dlls/user32/sysparams.c
index ab386a074e..043742a9bf 100644
--- 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;
 }