<div dir="auto"><div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Mar 5, 2022, 4:05 AM Jinoh Kang <<a href="mailto:jinoh.kang.kr@gmail.com">jinoh.kang.kr@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 3/5/22 02:47, Zebediah Figura wrote:<br>
> Right, but I think it's possible to trigger this assert now. For a rather pathological case, consider a client which calls set_async_direct_result on an async which is currently being serviced by a device driver and has not yet reached get_next_device_request.<br>
<br>
For that to be possible, something has to terminate the async so that set_async_direct_result does not reject it.<br>
The easiest way to do so is to cancel it, or make it timeout somehow.<br>
<br>
In fact, I think we have discovered an _existing_ bug: the client can use set_async_direct_result to interfere with cancelled asyncs (device-backed or not).<br>
<br>
Further investigation revealed some other bugs, which is tangentially related:<br>
<br>
1. An async may time out before the client calls set_async_direct_result to report back the status of initial I/O.  In this case, the timeout is ignored.<br>
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.<br>
   Note that this bug has existed *before* set_async_direct_result, and I think this one may need to be addressed first.<br>
<br>
So what I think we need here is a mechanism to tell alerted asyncs apart from irreversibly-terminated asyncs.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">To make it clear, it should be "restartable" vs. "non-restartable" asyncs. See below...</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This will solve all of the problems above:<br>
<br>
- It's no longer possible to call set_async_direct_result on a device async in the first place, since it will now reject irreversibly-terminated asyncs while still accepting alerted asyncs (which is not possible for device asyncs).<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Actually it's possible for devive asyncs to be in alerted state, but note that it is *not* supposed to be restartable.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
- Timeout and cancellation works as expected: set_async_direct_result will treat timed out asyncs as irreversibly-terminated.  (the client needs to be modified to handle this case explicitly, though.)<br>
<br>
Any thoughts?<br>
<br>
> <br>
> (And as mentioned, we should probably be calling async_set_initial_status() in async_set_result, in which case this could be triggered much less pathologically by the controlling process dying.)<br>
<br>
That has been split off as patch 2/8.<br>
<br>
<br>
-- <br>
Sincerely,<br>
Jinoh Kang<br>
</blockquote></div></div></div>