Windows/Event Loop: `GetQueuedCompletionStatus` should not panic in debug mode

[mjoerussell]

Zig Version

0.10.0-dev.37+3bd0575cf

Steps to Reproduce

When using GetQueuedCompletionStatus to process async network requests on Windows, cause a common error like disconnecting the peer. In this case, the function immediately panics with this error:

thread 20816 panic: unexpected error: Win32Error.NETNAME_DELETED

In a release build, this error is ignored entirely.

Reference: GetQueuedCompletionStatus.

Expected Behavior

GetQueuedCompletionStatus should return an error in all failure cases instead of panicking in debug builds, and doing nothing in release builds. The current behavior means that, when using this function, there's no way to detect that a certain error happened and then to handle it in an application-specific way.

Actual Behavior

GetQueuedCompletionStatus panics in all but a few cases in debug builds.

Example panic message:

thread 20816 panic: unexpected error: Win32Error.NETNAME_DELETED

[enkore]

Error needs to be handled here:

zig/lib/std/os/windows.zig

Line 361 in 9f9f215

switch (kernel32.GetLastError()) {

Is the approach of declaring unreachable error codes on Windows sound? As far as I remember MSDN never really said anything about what set of errors a given function can give you except maybe pointing out a few common cases.

[squeek502]

Is the approach of declaring unreachable error codes on Windows sound?

As I understand it, it's an imperfect temporary solution, and reaching an unexpected error/unexpected status on Windows should always be treated as a bug.

See PRs like #10495, #6465, #5535 which simply add a relevant switch case as they are found to be possible.

[andrewrk]

One thing that would be helpful would be someone to dive into Wine/ReactOS codebase for all these functions and find out all the possible error codes that can be returned.

[sskras]

@andrewrk, do you mean error codes generated by GetQueuedCompletionStatus() too?

[arBmind]

I digged a bit into the Wine source for GetQueuedCompletionStatus.
The method is basically forwarding the status of the queued message entry.
See: https://github.com/wine-mirror/wine/blob/1d178982ae5a73b18f367026c8689b56789c39fd/server/completion.c#L228

        msg = LIST_ENTRY( entry, struct comp_msg, queue_entry );
        /* snip */
        reply->status = msg->status;       

This status is later turned into the LastError.
See: https://github.com/wine-mirror/wine/blob/1d178982ae5a73b18f367026c8689b56789c39fd/dlls/kernelbase/sync.c#L1065

        if (iosb.u.Status >= 0) return TRUE;
        SetLastError( RtlNtStatusToDosError(iosb.u.Status) );
        return FALSE;

As an IoCompletionEvent can be triggered by many events the list will never be exhaustive.
I found the following possible NtStatus values in Wine (before translation to DosError)

• STATUS_INVALID_HANDLE
• STATUS_INVALID_PARAMETER
• STATUS_OBJECT_TYPE_MISMATCH
• STATUS_ACCESS_DENIED
• STATUS_PENDING
• STATUS_IO_TIMEOUT
• STATUS_TIMEOUT
• STATUS_END_OF_FILE
• STATUS_INVALID_USER_BUFFER
• STATUS_DISK_FULL
• STATUS_BUFFER_TOO_SMALL
• anything returned by errno_to_status

I could not find the authors NETNAME_DELETED in Wine. It's possible that the Wine implementation does not generate all status values for some of the Io Completion scenarios that an actual NT kernel would show.

[phatchman]

I had a look at ReactOS source code and I think it just confirms what @arBmind found. Basically, almost any possible error message that can be generated by performing IO on a handle can be returned here.

In ReactOS the errors are mostly generated in NtRemoveIoCompletion where it reads the error from a queue and returns it. The error is queued asynchronously by whichever IO operation was performed.

So, I just don't think it is possible to get a definitive list of errors to handle here. Even if we did, it seems like this list could be quite different between OS versions and could change without us having any knowledge.

I can only suggest we return some generic unknown error in these cases, which will require the user to call windows.GetLastError() if they actually care about the details.