https://bugs.winehq.org/show_bug.cgi?id=48946
Bug ID: 48946 Summary: Uplay crash when launching Watchdogs with esync patch applied Product: Wine-staging Version: 5.6 Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: -unknown Assignee: wine-bugs@winehq.org Reporter: guillaume.zin@gmail.com CC: leslie_alistair@hotmail.com, z.figura12@gmail.com Distribution: ---
Created attachment 66920 --> https://bugs.winehq.org/attachment.cgi?id=66920 Patch to esync, adds sanity checks in queue_rawinput_message
Hello,
I first decribed the problem here: https://github.com/lutris/wine/issues/25
With wine version 5.5 and 5.6 patched with esync, wineserver crashes most of the time (sometimes it doesn't) when Uplay is about to launch Watchdogs (when the dialog box appears to explain that disabling uplay overlay is not such a good idea, or when telling it is starting the game). It doesn't happen with vanilla wine.
Wine 5.6 + esync patch + -ggdb option during compilation:
Program received signal SIGSEGV, Segmentation fault. 0x000055baca61f208 in queue_rawinput_message (process=0x55bacb873200, user=0x7fff020150c0) at ../../wine/server/queue.c:1808 1808 ../../wine/server/queue.c: Aucun fichier ou dossier de ce type. (gdb) backtrace #0 0x000055baca61f208 in queue_rawinput_message (process=0x55bacb873200, user=0x7fff020150c0) at ../../wine/server/queue.c:1808 #1 0x000055baca619558 in enum_processes (cb=cb@entry=0x55baca61f140 <queue_rawinput_message>, user=user@entry=0x7fff020150c0) at ../../wine/server/process.c:1080 #2 0x000055baca620f21 in queue_mouse_message (req_flags=<optimized out>, sender=0x55bacbae5930, origin=1, input=0x55bacb588eb0, win=0, desktop=0x55bacb588110) at ../../wine/server/queue.c:1901 #3 req_send_hardware_message (req=0x55bacb588ea0, reply=0x7fff02015160) at ../../wine/server/queue.c:2608 #4 0x000055baca62a8d3 in call_req_handler (thread=thread@entry=0x55bacb588d50) at ../../wine/server/request.c:311 #5 0x000055baca62b902 in read_request (thread=thread@entry=0x55bacb588d50) at ../../wine/server/request.c:345 #6 0x000055baca6314b0 in thread_poll_event (fd=<optimized out>, event=1) at ../../wine/server/thread.c:319 #7 0x000055baca60a106 in fd_poll_event (event=<optimized out>, fd=<optimized out>) at ../../wine/server/fd.c:486 #8 main_loop_epoll () at ../../wine/server/fd.c:581 #9 0x000055baca60a37e in main_loop () at ../../wine/server/fd.c:947 #10 0x000055baca5f9989 in main (argc=1, argv=0x7fff02015948) at ../../wine/server/main.c:154
The attached patch, applied after esync patch, fixes the problem.
Guillaume
https://bugs.winehq.org/show_bug.cgi?id=48946
Rémi Bernon rbernon@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |rbernon@codeweavers.com
https://bugs.winehq.org/show_bug.cgi?id=48946
--- Comment #1 from Rémi Bernon rbernon@codeweavers.com --- The patch should probably go to the user32-rawinput-mouse staging series, but it may then require a rebase of the user32-rawinput-nolegacy series. There's going to be conflicts because two consecutive lines are touched although the merge is actually trivial.
https://bugs.winehq.org/show_bug.cgi?id=48946
Zebediah Figura z.figura12@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Fixed by SHA1| |8d4d0a840e6ce434483edd81acb | |3be90fd734e44 Resolution|--- |FIXED
--- Comment #2 from Zebediah Figura z.figura12@gmail.com --- This is fixed by https://github.com/wine-staging/wine-staging/commit/8d4d0a840e6ce434483edd81acb3be90fd734e44.
What does esync have to do with anything, though?
https://bugs.winehq.org/show_bug.cgi?id=48946
--- Comment #3 from Rémi Bernon rbernon@codeweavers.com --- Thanks! I don't know why esync is involved, but maybe it creates the conditions for the crash to happen. It crashed when there's no foreground_input thread on the desktop, I'm not sure if or how it's supposed to happen.
https://bugs.winehq.org/show_bug.cgi?id=48946
--- Comment #4 from Guillaume guillaume.zin@gmail.com --- esync is involved because I had the bug when applying the eventfd_synchronization patch. But looking at https://github.com/wine-staging/wine-staging/blob/master/patches/patchinstal..., I understand now that this patch applies a patch set including the user32_rawinput_mouse patch.
https://bugs.winehq.org/show_bug.cgi?id=48946
GloriousEggroll@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |GloriousEggroll@gmail.com
--- Comment #5 from GloriousEggroll@gmail.com --- This change seems to cause the window to close if you don't click into it so that it realizes it has focus. Happened when testing on both Watch Dogs 2 and Resident Evil 4. Removing this check:
+ if (!device->target && !desktop->foreground_input) + goto done;
Seems to work ok in both games from my end.
https://bugs.winehq.org/show_bug.cgi?id=48946
--- Comment #6 from GloriousEggroll@gmail.com --- What I mean is - if you launch the game, then click something else or otherwise cause the mouse to focus elsewhere, the game will try to open, then immediately close. If you click the black screen it produces shortly before initializing, it will sense that focus is active and continue to load as intended. Removing the check mentioned above allows the game to load regardless.
https://bugs.winehq.org/show_bug.cgi?id=48946
--- Comment #7 from Guillaume guillaume.zin@gmail.com --- I don't understand, it makes no sense: it means that sometimes, you enter the function with device->target and desktop->foreground_input = 0. It should crash on "device->target ? device->target : desktop->foreground_input->active" without the sanity check, isn'it ?
https://bugs.winehq.org/show_bug.cgi?id=48946
--- Comment #8 from Guillaume guillaume.zin@gmail.com --- Created attachment 66953 --> https://bugs.winehq.org/attachment.cgi?id=66953 user32_rawinput_mouse cleaner patches
These patches makes the code more readily understandable I think, but I'm not sure it will fix GloriousEggroll troubles with Watch Dogs 2 and Resident Evil 4. One patch is to apply on vanilla Wine 5.6, the other on current staging code.
https://bugs.winehq.org/show_bug.cgi?id=48946
--- Comment #9 from Rémi Bernon rbernon@codeweavers.com --- I don't think they are right though. If the registered rawinput device has no target window, the messages should be sent to the foreground window. With your changes, "thread = get_window_thread( device->target )" will be NULL in this case and the message will be discarded.
Regarding GloriousEggroll issue, I'm not sure to see how that's ever related to rawinput. If the window is not foreground it isn't supposed to receive any rawinput message, unless RIDEV_INPUTSINK is also implemented, which is done in a different patch series. I will investigate a bit but I'm on something else right now, so it may take a while.
https://bugs.winehq.org/show_bug.cgi?id=48946
--- Comment #10 from Guillaume guillaume.zin@gmail.com --- In my understanding, get_window_thread is used on device->target then on desktop->foreground_input->active just to compare with message process and check that the message is the for the foreground thread, otherwise it is discarded. Thread object is not used afterwards. But the code is odd in the current state, and I'm clearly not an expert.
https://bugs.winehq.org/show_bug.cgi?id=48946
--- Comment #11 from Rémi Bernon rbernon@codeweavers.com --- The code iterates the process list on every rawinput message to see which one is interested. Then, every process can register for rawinput message and may specify a target window, or let the system send the message to the foreground window if it belongs to the given process.
The first implements that by getting the target thread for the device (either the owner of the target window, or the foreground thread), then it compares with the process that is currently checked, and if the thread is from another process bails out - this other process will get its own iteration step and receive the message if it is interested.
Then, the second if implements a different check where, if the target thread is not from the foreground process (the process to which belongs the foreground window), then we should only sent the messages if a specific flag is used, which is first unimplemented -not supporting RIDEV_INPUTSINK flag / WM_INPUTSINK messages, and then later implemented in a different patch series.
https://bugs.winehq.org/show_bug.cgi?id=48946
--- Comment #12 from Guillaume guillaume.zin@gmail.com --- I see, so forget the user32_rawinput_mouse cleaner patches, it might break work in progress. Thank you for the explanations.
https://bugs.winehq.org/show_bug.cgi?id=48946
--- Comment #13 from GloriousEggroll@gmail.com --- I did a double check on this - the fix is fine and correct. I had another upstream pending patch applied that's doing some really funky stuff with clipping that doesnt seem to work properly:
(https://bugs.winehq.org/show_bug.cgi?id=48772) for reference
https://bugs.winehq.org/show_bug.cgi?id=48946
Zebediah Figura z.figura12@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #14 from Zebediah Figura z.figura12@gmail.com --- Closing bugs fixed in wine-staging 5.7.