Re: [PATCH v3 0/1] MR499: server: Allow cancelling alerted asyncs.

[Jinoh Kang (＠iamahuman)]

> This works but it looks fragile. I haven't actually checked, but is there any other way that the same async can be terminated twice?

Yes.  Consider the following scenario on `async`:

1. I/O timeout is configured for `async` via an `async_set_timeout()` call.
1. `async` enters alerted state.
2. An application calls `NtCancelIoFileEx` with `async`'s user IOSB.
3. `async_terminate( async, STATUS_CANCELLED )` is called.
4. The timeout expires.
5. `async_terminate( async, async->timeout_status );` is called.  (`async->timeout_status` is usually `STATUS_TIMEOUT`.)

Furthermore, I/O cancellation and timeout can race with other async termination requests.
> If so, is it correct to buffer the second termination (and not, say, buffer a third?)

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.
> If not, should we make this explicitly a boolean flag ('canceled' I guess?)

It is possible to introducing a new boolean flag, but the flag cannot replace `terminate_status`.  `terminate_status` can be any arbitrary value passed as the third parameter of `async_set_timeout`.

Alternatively, we can rename `terminate_status` to `cancel_status` or `cancellation_status` (may be a bit misleading: is a timeout a cancellation?)
> Or at least add some asserts to communicate exactly when this is supposed to happen?

Right now I don't see any additional invariants that the new code paths would expect.  `terminate_status` simply means the first status that the async was terminated with, other than `STATUS_ALERTED`.  Termination can happen regardless of whether the async is in alerted state, but `terminate_status` would be used in `async_set_result` only if `async->alerted`.  Can you suggest what the assertions would be?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/499#note_30342