On Tue Aug 26 23:54:18 2025 +0000, Matteo Bruni wrote:
+ else if (status == STATUS_INVALID_HANDLE) + { + return status; + } + + /* TODO: Add a test for the timing of setting IOSB values? It's tricky + * since we probably have to check from a different thread
without using
* win32 IPC for synchronization to avoid syncing on system APC
* execution. */
- io_status->Status = status;
- io_status->Information = 0;
- return status;
I guess it doesn't make a difference here, but more generally the iosb
is set on !NT_ERROR(status), so that may be more idiomatic to write instead of checking for STATUS_INVALID_HANDLE specifically. Annoyingly it does matter here: one of the tests in 7/8 fails with the change (specifically, the cancel operation IOSB is in fact updated for STATUS_NOT_FOUND).
I'm attaching some new ntoskrnl tests which pass for me on up to date Windows 10 and 11. There's room for cleanup but I think they answer most of the questions you had; I'm curious to know if you think this is convincing enough or otherwise you'd like some other piece tested. Or anything else, really (e.g. maybe I'm taking the wrong conclusions from those test results...)
It looks like only `CancelIo()`, not `CancelIoEx()`, waits for the operations to complete. It does wait for completion even if the operations aren't actually cancelled but eventually end regularly on their own.
On the other hand, `CancelSynchronousIo()` really wants the completion routine itself to call `IoCompleteRequest()`. The values set in the IOSB `Status` and `Information` fields are apparently not particularly important, or at least nothing breaks immediately if those are set to unexpected values.
The thread ID check I added in `cancel_ioctl_irp()` does seem to strongly suggest that the cancel routine is called synchronously. Not sure if this test in particular is "safe" for upstream.
Assuming all this makes sense, it looks to me that the approach from this MR is okay once I limit it to `CancelIo()`, leaving `CancelIoEx()` to the old mechanism. Luckily there is no concern with multiple "cancels" having to wait on the same operations. I guess I could also use `async_cancel` for `CancelSynchronousIo()` and get closer to native, although that's never going to be quite right until the whole async cancellation / completion becomes more synchronous.
[ntoskrnl-cancel-tests.txt](/uploads/6067830a1d7b7355df5c048a5d67c4df/ntoskrnl-cancel-tests.txt)