On 3/8/22 09:33, Jinoh Kang wrote:
In fact, I'm somewhat concerned that if we let synchronous I/O time out before it even had a chance to start, we would break applications that expect it to work.
- There is a time window where alerted asyncs ignore IoCancel/IoCancelEx requests. If async_terminate( async, STATUS_ALERTED ) is called, the async will ignore any cancellation requests until the APC_ASYNC_IO routine returns.
Note that this bug has existed *before* set_async_direct_result, and I think this one may need to be addressed first.
Yes, this has been a bug for a while. I don't remember if this was a problem before the async rework in 7.0. We probably need to "buffer" the cancellation request somehow, and process it once the async is restarted. (As above, I don't think we want to cancel the async if the client is going to report success to us.)
Sounds like a great idea. Not sure which one is the best way to implement it, though. A few options come to my mind:
- Allow async_terminate() to override status if async->alerted is set, even if it is already terminated.
We used to have this (before a5b6e90d48e), but I don't think this actually helps. If the client restarts we're back where we started. We could account for it there but I don't much like the added complexity.
- Add a separate status field for buffering cancellation requests.
This seems more desirable to me. We'll need to do the same thing with timeouts.
Perhaps add cancel_status in struct async? It would hold STATUS_TIMEOUT or STATUS_CANCELLED (or whatever).
Sure, that sounds reasonable to me.