[PATCH 0/1] MR2242: win32u: Do not set last error in wait_message() for zero count.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54405 Also fixes Assassin Creed hanging on start (for a similar reason: it doesn't expect a spurious -1 last error). There is already a fix attempt sent by Ivan Chikish in !2067 (https://gitlab.winehq.org/wine/wine/-/merge_requests/2067). But I suppose it may be fixed simpler like in this patch. wait_message() with zero count is only called from NtUserPeekMessage(). I think even if for some reason driver's MsgWaitForMultipleObjectsEx returns a real error code we still don't want to set last error here when called from PeekMessage(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2242
From: Paul Gofman <pgofman(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54405 --- dlls/win32u/message.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index ed4198991cf..60598bbc86a 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -2121,7 +2121,7 @@ static DWORD wait_message( DWORD count, const HANDLE *handles, DWORD timeout, DW mask, flags ); if (HIWORD(ret)) /* is it an error code? */ { - RtlSetLastWin32Error( RtlNtStatusToDosError(ret) ); + if (count) RtlSetLastWin32Error( RtlNtStatusToDosError(ret) ); ret = WAIT_FAILED; } if (ret == WAIT_TIMEOUT && !count && !timeout) NtYieldExecution(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2242
Wait, how can the wait fail if we're not passing any handles? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2242#note_24910
There is an explanation in the linked MR which I think makes sense. Drivers' (winex11, winemac) MsgWaitForMultipleObjectsEx returns 'count - 1' if process_events() adds some events. Which doesn't work exactly straightforward if count is 0. The original MR suggests changing the drivers' convention around return value (maybe is a bit wrong way, although that should still be possible to do right). It seems to me that it maybe doesn't worth it (as driver's MsgWaitForMultipleObjectsEx convention is ok for a sane case of non-zero events count). So I am suggesting to do it simpler and rather handle this special peekmessage case with 0 handle count. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2242#note_24917
I guess if it were up to me I'd adjust the return value convention, but failing that perhaps this change deserves better documentation in the code? It's not at all clear that's why the check is there... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2242#note_24922
Regarding convention, does it really worth it to return some special value or check for 0 count in each driver for this special case? It seems to me it would make things uglier, count - 1 is not a special value, for all the other cases it indicates that the wait succeeded for the last object which is always queue handle. Instead of checking of zero count I could as well (instead) check for ~0 status, not sure if adding a special one for this case will make anything easier? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2242#note_24925
Well, actually, it seems like there is no need to change any convension, we can just check for 0 count in driver and return 0 in that case which probably will be more straightforward (I checked that there is no other usage of driver's MsgWaitForMultipleObjectsEx for which that would introduce ambiguity). I will resend that way. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2242#note_24927
participants (3)
-
Paul Gofman -
Paul Gofman (＠gofman)
-
Zebediah Figura (＠zfigura)