Re: [PATCH v12 0/3] MR5060: win32u, explorer, winex11.drv: Restore display settings on process exit.

[Zhiyi Zhang (＠zhiyi)]

On Fri Oct 18 13:56:14 2024 +0000, Rémi Bernon wrote:
> Do you recall why b3eb55227aef198a637f80a2fa0bce74aa0a7190 is actually
> required? Can't we just wait until the RRNotify events are received and
> processed from whichever thread listens to them?
> Fwiw it is causing some rare deadlocks with this kind of pattern:
> - thread A holds `display_lock` and wants `user_lock`:
> #4  0x00007f3f11236940 in user_lock () at ./dlls/win32u/sysparams.c:295
> #5  0x00007f3f11246551 in get_user_handle_ptr (type=1, handle=0x10020)
> at ./dlls/win32u/window.c:90
> #6  get_win_ptr (hwnd=0x10020) at ./dlls/win32u/window.c:249
> #7  0x00007f3f11246e2b in get_window_thread (hwnd=0x10020, process=0x0)
> at ./dlls/win32u/window.c:320
> #8  0x00007f3f1124ce8d in NtUserCallHwndParam (hwnd=hwnd@entry=0x10020,
> param=param@entry=0, code=code@entry=16) at ./dlls/win32u/window.c:5892
> #9  0x00007f3f10c6803f in NtUserGetWindowThread (process=0x0,
> hwnd=0x10020) at ./include/ntuser.h:1431
> #10 xrandr14_device_change_handler (hwnd=0x10020, event=<optimized out>)
> at ./dlls/winex11.drv/xrandr.c:1221
> #11 0x00007f3f10c3535f in call_event_handler
> (display=display@entry=0x55558c2bdcd0, event=event@entry=0x7ffffe0f9a80)
> at ./dlls/winex11.drv/event.c:460
> #12 0x00007f3f10c37e27 in process_events (display=<optimized out>,
> filter=filter@entry=0x7f3f10c65c30 <filter_rrnotify_event>,
> arg=<optimized out>) at ./dlls/winex11.drv/event.c:518
> #13 0x00007f3f10c68f4f in process_rrnotify_events () at ./dlls/winex11.drv/xrandr.c:1270
> #14 xrandr14_get_id (device_name=<optimized out>, is_primary=<optimized
> out>, id=0x7ffffe0f9c98) at ./dlls/winex11.drv/xrandr.c:1288
> #15 0x00007f3f10c33fe7 in X11DRV_UpdateDisplayDevices
> (device_manager=0x7f3f1129f700 <device_manager>, param=0x7ffffe0fa680)
> at ./dlls/winex11.drv/display.c:453
> #16 0x00007f3f11236ba8 in update_display_devices (ctx=0x7ffffe0fa680) at ./dlls/win32u/sysparams.c:2027
> #17 update_display_cache (force=force@entry=1) at ./dlls/win32u/sysparams.c:2072
> #18 0x00007f3f11238e5f in display_mode_changed (broadcast=1) at ./dlls/win32u/sysparams.c:3473
> #19 0x00007f3f11239cd2 in apply_display_settings
> (target=target@entry=0x0, devmode=devmode@entry=0x0,
> hwnd=hwnd@entry=0x0, flags=flags@entry=0, lparam=lparam@entry=0x0) at ./dlls/win32u/sysparams.c:3566
> #20 0x00007f3f1123d57d in NtUserChangeDisplaySettings
> (devname=0x7ffffe2fe600, devmode=0x0, hwnd=0x0, flags=0, lparam=0x0) at ./dlls/win32u/sysparams.c:3583
> #21 <signal handler called>
> #22 0x00006ffffe08bd74 in NtUserChangeDisplaySettings () from /home/rbernon/Code/build-wine-remote/build64/dlls/win32u/x86_64-windows/win32u.dll
> #23 0x00006ffffe0dfdc3 in ChangeDisplaySettingsExW
> (devname=devname@entry=0x0, devmode=devmode@entry=0x0,
> hwnd=hwnd@entry=0x0, flags=flags@entry=0, lparam=lparam@entry=0x0) at ./dlls/user32/sysparams.c:549
> #24 0x00006ffffa591779 in wined3d_restore_display_modes
> (wined3d=0x7ffffe2fec48) at ./dlls/wined3d/directx.c:1636
> #25 0x00006ffffa59d54e in wined3d_swapchain_activate
> (swapchain=0x7fffff26eaa0, activate=0, activate@entry=32767) at ./dlls/wined3d/swapchain.c:1899
> - thread B holds `user_lock` and wants `display_lock`:
> #4  0x00007f3f1123adcc in lock_display_devices () at ./dlls/win32u/sysparams.c:2100
> #5  get_virtual_screen_rect (dpi=dpi@entry=0, type=<optimized out>) at ./dlls/win32u/sysparams.c:2520
> #6  0x00007f3f112439a7 in NtUserCallTwoParam
> (arg1=arg1@entry=140737480547808, arg2=arg2@entry=0, code=code@entry=10)
> at ./dlls/win32u/sysparams.c:6758
> #7  0x00007f3f10c5a208 in NtUserGetVirtualScreenRect
> (type=MDT_EFFECTIVE_DPI) at ./include/ntuser.h:1155
> #8  X11DRV_ReleaseDC (hwnd=<optimized out>, hdc=0x22010048) at ./dlls/winex11.drv/window.c:2700
> #9  0x00007f3f111842bd in release_dce (dce=dce@entry=0x55558c2a97b0) at ./dlls/win32u/dce.c:887
> #10 0x00007f3f11184810 in release_dce (dce=0x55558c2a97b0) at ./dlls/win32u/dce.c:1138
> #11 0x00007f3f11187118 in invalidate_dce (win=win@entry=0x7f3ec00aa020,
> old_rect=old_rect@entry=0x7fffff88dfd0) at ./dlls/win32u/dce.c:1201
> #12 0x00007f3f1124e25c in apply_window_pos (hwnd=hwnd@entry=0x6004e,
> insert_after=insert_after@entry=0x0, swp_flags=6207,
> swp_flags@entry=6175, new_surface=new_surface@entry=0x7f3ec009f210,
> new_rects=new_rects@entry=0x7fffff88e170,
> valid_rects=valid_rects@entry=0x7fffff88e150) at ./dlls/win32u/window.c:2084
> #13 0x00007f3f112519fa in update_window_state (hwnd=0x6004e) at ./dlls/win32u/window.c:4523
> #14 0x00007f3f1120dedd in handle_internal_message (hwnd=<optimized out>,
> msg=<optimized out>, wparam=0, lparam=<optimized out>) at ./dlls/win32u/message.c:2125
> #15 0x00007f3f112122a1 in peek_message (msg=msg@entry=0x7fffff88e9f0,
> filter=filter@entry=0x7fffff88e9d0) at ./dlls/win32u/message.c:2923
> #16 0x00007f3f112138c8 in NtUserPeekMessage (msg_out=0x7fffffa8ff00,
> hwnd=<optimized out>, first=<optimized out>, last=<optimized out>,
> flags=<optimized out>) at ./dlls/win32u/message.c:3242
> #17 <signal handler called>
> #18 0x00006ffffe08cd54 in NtUserPeekMessage () from /home/rbernon/Code/build-wine-remote/build64/dlls/win32u/x86_64-windows/win32u.dll
> #19 0x00006ffffe113965 in PeekMessageW (flags=1024, last=<optimized
> out>, first=<optimized out>, hwnd=<optimized out>,
> msg_out=0x7fffffa8ff00) at ./dlls/user32/message.c:762
> #20 PeekMessageA (msg=0x7fffffa8ff00, hwnd=<optimized out>,
> first=<optimized out>, last=<optimized out>, flags=1) at ./dlls/user32/message.c:772
>
> This happens randomly, perhaps more frequently since
> 2b32c2859166e5542012b517cdef4699d8bb322c, but `update_window_state`
> could already be called before in other circumstances, and it should not
> cause a deadlock.
> More specifically, I don't think we should try to acquire `user_lock`
> with `display_lock` held, only the other way around.

If I remember correctly, it's for other threads other than the explorer's main thread to see the new display mode immediately. Without it, a rather large delay is causing tests to fail.