Add DeviceIoControlAsync helper method

[qmfrederik]

This is a draft PR, to make the suggestion of #500 a bit more tangible. In this case, it adds an async wrapper around DeviceIOControl like this:

Task<int> DeviceIoControlAsync(
            SafeObjectHandle hDevice,
            int dwIoControlCode,
            Memory<byte> inBuffer,
            Memory<byte> outBuffer,
            CancellationToken cancellationToken)

The bulk of the 'glue' to convert a kernel method which returns Win32ErrorCode.ERROR_IO_PENDING to an async method is in the DeviceIOControlOverlapped class. It ultimately registers a callback, which operates on a TaskCompletionSource.

In its current shape, the method task Memory<byte> rather than void*. The reason is C# doesn't let you mix unsafe and async in the same method; so if you want to call this method in an async manner, have to avoid void*.
This does mean targeting netstandard2.1.

[AArnott]

This does mean targeting netstandard2.1.

It doesn't, actually. I pushed a commit to demonstrate.

[AArnott]

I think this will be a fine fit.

But I'd like to explore reducing memory pressure by using NativeOverlapped (a struct) directly instead of the Overlapped class. Also consider using ValueTask<uint> instead of Task<uint>.

[AArnott]

We shouldn't create files based on an assumption of the current directory, since xunit doesn't guarantee a particular current directory.

[AArnott]

Perhaps create a file under the temp directory (and delete it later)?

[AArnott]

Fix up the created file path, and promote your PR out of draft when you're ready to merge. :)

[AArnott]

Perhaps create a file under the temp directory (and delete it later)?

[qmfrederik]

Thanks, @AArnott . I rebased the PR, changed the DeviceIoControlAsync to return a ValueTask<uint>, added a unit tests for the code path where the DeviceIoControl completes synchronously and DeviceIoControl errors out, updated the XML code comments and used a temporary file.

I looked at replacing Overlapped (the class) by NativeOverlapped (the struct). I assumed it'd mainly be a matter of setting Overlapped->hEvent so the kernel can invoke .NET code when the operation completes.

The implementation of Overlapped and NativeOverlapped look very specialized. I'm not sure whether that's because Overlapped tries to be generic and we can specialize; or because of perf concerns.

Net, if you want the further optimize this, I'll need some hints. Since it's an implementation detail (no changes to the public API), we could perhaps do that on a future PR.

[qmfrederik]

Out of curiosity, since this constructor (and the properties below) are defined on a class which is private to Kernel32, is there any difference between marking it as public or internal?

[AArnott]

There are subtle differences in Reflection, but the main reason is because public triggers "public API review" mode in my mind as public APIs are very hard to change later, and these are in fact not public APIs. The C# public keyword in private or internal types are in fact not public at all (unless they also implement an interface). So it's a misleading keyword to use, and makes code reviews more expensive. So I prefer internal for members of non-public types instead of public.

[AArnott]

The use of the .NET Overlapped class is prohibiting the definition of your async helper method for store apps. Targeting uap10.1 would fix this, or removing use of that class. But as you say, there's no breaking API impact to fixing this later, so we can complete this as-is. Thanks!