Re: [PATCH v12 0/3] MR5060: win32u, explorer, winex11.drv: Restore display settings on process exit.
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: 1) 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(a)entry=0x10020, param=param(a)entry=0, code=code(a)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(a)entry=0x55558c2bdcd0, event=event(a)entry=0x7ffffe0f9a80) at ./dlls/winex11.drv/event.c:460 #12 0x00007f3f10c37e27 in process_events (display=<optimized out>, filter=filter(a)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(a)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(a)entry=0x0, devmode=devmode(a)entry=0x0, hwnd=hwnd(a)entry=0x0, flags=flags(a)entry=0, lparam=lparam(a)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(a)entry=0x0, devmode=devmode(a)entry=0x0, hwnd=hwnd(a)entry=0x0, flags=flags(a)entry=0, lparam=lparam(a)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(a)entry=32767) at ./dlls/wined3d/swapchain.c:1899 ``` 2) 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(a)entry=0, type=<optimized out>) at ./dlls/win32u/sysparams.c:2520 #6 0x00007f3f112439a7 in NtUserCallTwoParam (arg1=arg1(a)entry=140737480547808, arg2=arg2(a)entry=0, code=code(a)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(a)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(a)entry=0x7f3ec00aa020, old_rect=old_rect(a)entry=0x7fffff88dfd0) at ./dlls/win32u/dce.c:1201 #12 0x00007f3f1124e25c in apply_window_pos (hwnd=hwnd(a)entry=0x6004e, insert_after=insert_after(a)entry=0x0, swp_flags=6207, swp_flags(a)entry=6175, new_surface=new_surface(a)entry=0x7f3ec009f210, new_rects=new_rects(a)entry=0x7fffff88e170, valid_rects=valid_rects(a)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(a)entry=0x7fffff88e9f0, filter=filter(a)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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5060#note_85456
participants (1)
-
Zhiyi Zhang (＠zhiyi)