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 --- 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 ab386a074e8..043742a9bfa 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; }