> I don't think so. `async_terminate()` is currently no-op if the async has already been terminated with a status other than `STATUS_ALERTED`, and the patch maintains this invariant.
Right, that's the bit I failed to think through.
> It is possible to introduce a new boolean flag, but the flag cannot replace `terminate_status`. `terminate_status` can be any arbitrary value passed to the third parameter of `async_set_timeout`.
Ah, and I missed that that was a possibility, although, in my defense, it's not stated in the patch subject either. I.e. this patch doesn't just fix the race where an alerted async is canceled, it also fixes the similar race where it times out.
So yes, I think the patch makes sense as is. Maybe a comment to yet again overdocument asyncs wouldn't hurt? I easily see myself looking at code like this and having to ask myself, "when exactly is terminate_status set?" and the answer is "if (and only if?) the async is canceled or times out while it's alerted", which is nice to have documented and not have to puzzle through later.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/499#note_30389
--
v4: kernelbase: Implement activation context switching for fibers.
ntdll: Implement RtlFreeActivationContextStack().
ntdll: Use ActivationContextStackPointer instead of referencing ActivationContextStack directly.
ntdll: Store current activation context stack pointer into a local variable.
ntdll: Factor out reading current activation context into a helper function.
kernel32/tests: Test for activation context switching between fibers.
https://gitlab.winehq.org/wine/wine/-/merge_requests/2435
MSVC doesn't support "name...".
---
msvc also supports __VA_ARGS__ only if /Zc:preprocessor is set to make
its preprocessor "standards conformant" instead of doing whatever msvc
used to do since aeons.
--
v2: d3dx9/tests: Use __VA_ARGS__ instead of GCCs named variadics.
d3dx9: Use __VA_ARGS__ instead of GCCs named variadics.
https://gitlab.winehq.org/wine/wine/-/merge_requests/2666
> I must be missing something, when can dest == src?
If `ActivationContextStackPointer` already equals `dest` (fiber data or TEB). I think it's not possible currently, unless the user tinkers with undocumented fields (which we don't need to support right now).
> Aren't we always moving from the fiber data to the TEB in ConvertFiberToThread()?
Or in the other direction in case of `ConvertThreadToFiber()`, but yes, `dest != src` in that case too.
I'll remove the check, since I realized it would not be necessary even if `dest == src` was indeed true. Thank you for your review.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/2435#note_30358
On Mon Apr 17 21:49:22 2023 +0000, Zebediah Figura wrote:
> What's this for?
This is a common idiom for suppressing the `-Wunused-parameter` warning. However, Wine doesn't enable it by default, even in maintainer mode.
I'll remove this line, since compiling Wine with `-Wunused` would already emit unused parameter warnings. Just an old habit of mine, I guess.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/2435#note_30356