Consistently present absent stdio handles on Windows as NULL handles.

[sunfishcode]

This addresses #90964 by making the std API consistent about presenting
absent stdio handles on Windows as NULL handles. Stdio handles may be
absent due to #![windows_subsystem = "windows"], due to the console
being detached, or due to a child process having been launched from a
parent where stdio handles are absent.

Specifically, this fixes the case of child processes of parents with absent
stdio, which previously ended up with stdin().as_raw_handle() returning
INVALID_HANDLE_VALUE, which was surprising, and which overlapped with an
unrelated valid handle value. With this patch, stdin().as_raw_handle()
now returns null in these situation, which is consistent with what it
does in the parent process.

And, document this in the "Windows Portability Considerations" sections of
the relevant documentation.

[rust-highfive]

r? @dtolnay

(rust-highfive has picked a reviewer for you, use r? to override)

[joshtriplett]

What do you mean by "which is consistent with what it does in the parent process"? It sounds like GetStdHandle returns INVALID_HANDLE_VALUE for this case.

[sunfishcode]

What do you mean by "which is consistent with what it does in the parent process"? It sounds like GetStdHandle returns INVALID_HANDLE_VALUE for this case.

In a parent with a detached console, its GetStdHandle returns NULL, and Rust's Stdin::as_raw_handle() returns NULL.

Currently, in the child of such a parent, Stdin::as_raw_handle() returns INVALID_HANDLE_VALUE. With this patch, it will return NULL, the same as Stdin::as_raw_handle does in the parent process.

[dtolnay]

@rust-lang/libs-api:
@rfcbot fcp merge

This PR concerns the behavior of the stable AsRawHandle on Windows. Previously a process with absent stdio would return null ptr for its stdio handles, but child processes of that process would return INVALID_HANDLE_VALUE (which is !0) for its stdio handles. This discrepancy is surprising and problematic because INVALID_HANDLE_VALUE aliases the value of the current process handle.

I haven't read all of #90964, though hopefully one of my teammates more knowledgeable on Windows will take a closer look. The justification in #90964 (comment) seems compelling to me. Though be aware this solution diverges from the preference of a set of anonymous Microsoft consultees.

[rfcbot]

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se
• @yaahc

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[joshtriplett]

Discussed some minor tweaks for readability of the repeated handle usage with @sunfishcode (adding a helper function), but otherwise LGTM.

@rfcbot reviewed

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[sunfishcode]

Looking at the code, it looks like those redundant checks are not actually needed. I've now submitted a patch which removes them.

[pravic]

INVALID_HANDLE_VALUE aliases the current process handle

Well, they are totally different types. No one would try to pass io::RawHandle where io::RawSocket is expected or where a process handle is expected.

[sunfishcode]

In Windows, the current process handle, as returned by GetCurrentProcess has type HANDLE, aka io::RawHandle here, which is the same as the return type of eg. CreateFileW.

SOCKET, aka io::RawSocket here, on the other hand, is a different type.

[pravic]

And so does a dozens of other types that are also an alias (or a newtype if compiled with -DSTRICT) of HANDLE: https://docs.microsoft.com/en-us/windows/win32/winprog/windows-data-types

So, in fact it should be a newtype in Rust as well.

Okay, to be more formal: while both GetCurrentProcess and CreateFile (and CreateEvent, etc) have the same types (HANDLE), in fact they are different handle types and no one familiar with winapi would try to use them interchangeably.

[sunfishcode]

Some of the types linked there using HANDLE are so different that they don't even use CloseHandle. We already do document that it's not valid to use OwnedHandle/BorrowedHandle to hold those kinds of handles.

Beyond that though, OwnedHandle, BorrowedHandle, and AsHandle are intended to be used with both file handles and process handles, for three reasons:

• Rust already uses the same RawHandle/AsRawHandle/IntoRawHandle types and traits for both processes and files, so it's consistent with existing APIs in Rust
• Both process handles and file handles can be passed to CloseHandle, GetHandleInformation, SetHandleInformation, DuplicateHandle, and others, so they may be different, but they're not completely different.
• Rust also does have distinct higher-level types for different kinds of handles, such as std::fs::File for file(-like) handles, and std::process::Child for process handles, and these are what most Rust code uses most of the time, so it isn't as important to make a static type distinction at the OwnedHandle/etc. level.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[dtolnay]

@bors r+

[bors]

📌 Commit 7ddf41c has been approved by dtolnay