[PATCH v4 1/8] server: Actually set initial status in set_async_direct_result handler.

[Zebediah Figura]

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.
>>>>
>>>>> 2. 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.