Should getting a `BorrowedHandle` from `Stdin`, `Stdout` or `Stderr` return a `Result` on Windows?

[ChrisDenton]

On Windows, the standard library's function for getting a stdio handle looks like this:

pub fn get_handle(handle_id: c::DWORD) -> io::Result<c::HANDLE> {
    let handle = unsafe { c::GetStdHandle(handle_id) };
    if handle == c::INVALID_HANDLE_VALUE {
        Err(io::Error::last_os_error())
    } else if handle.is_null() {
        Err(io::Error::from_raw_os_error(c::ERROR_INVALID_HANDLE as i32))
    } else {
        Ok(handle)
    }
}

Note that it only returns a handle if one is set, otherwise it returns an error.

In contrast, the public AsRawHandle stdio implementation ignores errors returned by GetStdHandle and just uses the returned value, whatever that may be.

impl AsRawHandle for io::Stdin {
    fn as_raw_handle(&self) -> RawHandle {
        unsafe { c::GetStdHandle(c::STD_INPUT_HANDLE) as RawHandle }
    }
}
// ...and similar for `Stdout` and `Stdin`.

The Safe I/O RFC introduced new types for managing handles. The AsHandle trait is intended to be a drop in replacement for the old AsRawHandle trait but returns a BorrowedHandle instead of a RawHandle.

I personally don't think AsHandle should be implemented for stdio types. Instead a function with a signature similar to this should be implemented:

fn try_as_handle(&self) -> io::Result<BorrowedHandle<'_>>;

It would work similarly to the internal get_handle function in that it will return an error if there is no handle to return.

---

Reasons for a try_as_handle() function instead of implementing AsHandle on stdio types:

• The error is surfaced at its origin.
• It gives users the correct mental model when thinking about Windows' stdio (especially since it differs from Unix).
• A BorrowedHandle should be what the name implies: a borrow of a handle. It should not be an error sentinel value (which may overlap with an actual handle value).

Reasons not to do this:

• Using a null or INVALID_HANDLE_VALUE will probably lead to an error in any case so it's unclear how much of an issue this is in practice.
• It makes it harder to update code if AsRawHandle and AsHandle aren't implemented for all the same types.
• Given the above, is changing this really worth the trade-off?

See also: I/O Safety Tracking Issue (#87074) and a previous discussion on internals.

[programmerjake]

technically you can also get an error on unix, e.g. the stdio file descriptor is closed.

[ChrisDenton]

Right but that's a different sort of error. If the stdin file descriptor is closed then the stdin file descriptor itself still exists. I mean fd 0 doesn't ever stop being fd 0. It's a static value.

On Windows the value isn't static. It's retrieved at runtime, which can fail.

[jstarks]

On Windows the value isn't static. It's retrieved at runtime, which can fail.

I wouldn't quite use the word fail. Internally in Windows, there's a struct with one value for each of the three standard handles. GetStdHandle will return whatever values happen to be there, which could be NULL if the process was launched without stdio handles, or could be INVALID_HANDLE_VALUE if someone set the handles as such via SetStdHandle (no validation is or realistically can be performed by this routine).

Notably GetStdHandle only explicitly returns INVALID_HANDLE_VALUE if you pass an invalid argument, i.e. you pass something other than STD_{INPUT,OUTPUT,ERROR}_HANDLE.

So realistically, the only case worth considering is when the process was launched without stdio handles (e.g. a GUI process). Do you want to return an explicit error in that case, or is a BorrowedHandle containing NULL acceptable? Or maybe Option<BorrowedHandle> best captures the semantics?

[ChrisDenton]

Sure Option<BorrowedHandle> would work. But I'd note that Rust itself explicitly sets INVALID_HANDLE_VALUE in some cases when spawning a new process so it would be strange if Rust code didn't handle that. This can also occur due to an uncaught error in the parent process which they inadvertently pass on.

I do think that BorrowedHandle should represent an actual handle. This makes it useful to avoid having to defensively check it's null or whatever when it's given as input.

[programmerjake]

Right but that's a different sort of error. If the stdin file descriptor is closed then the stdin file descriptor itself still exists. I mean fd 0 doesn't ever stop being fd 0. It's a static value.

I'd argue that returning a closed fd is the equivalent of returning Box::from_raw(1usize as *mut u8) aka. immediate UB. Therefore, APIs should never return a closed fd in Owned/BorrowedFd

[jstarks]

But I'd note that Rust itself explicitly sets INVALID_HANDLE_VALUE in some cases when spawning a new process so it would be strange if Rust code didn't handle that.

Hmm. I think passing NULL in that case would be more appropriate.

[ChrisDenton]

As do I. But even if we change it now we still have to live with older Rust programs that don't. And in any case I doubt that Rust standard library is the only code that does this.

[jstarks]

But maybe to be defensive it would be wise to normalize both NULL and INVALID_HANDLE_VALUE into the same state, whether that be BorrowedHandle(NULL), None, or Err(_).

[ChrisDenton]

Yeah. That's what the standard library does internally. It makes them both an Err.

[sunfishcode]

I'm confused about how the get_handle code quoted above corresponds to how std seems to behave in practice.

I just tried an experiment. It seems that in windows_subsystem = "windows" mode, Stdout::as_raw_handle() always returns NULL. It returns NULL in the windows_subsystem = "windows" parent process, in a windows_subsystem = "windows" child launched from the parent, and also in a normal child launched from this parent. And in all cases, writing to the standard output stream silently succeeds, without printing anything to the console.

I've so far not been able to find any place where accessing Stdout with a NULL handle produces an observable error, or any cases where INVALID_HANDLE_VALUE is passed into the child as a result of a missing handle. Am I looking in the wrong places?

[ChrisDenton]

It's hard to judge because you seem to be running programs through cargo?

In the case of using stdio types themselves, I wouldn't expect them to fail. If the std handle is NULL or INVALID_HANDLE_VALUE it silently pretends any operations are successful without actually calling into the Windows API. See

rust/library/std/src/io/stdio.rs

Lines 189 to 194 in 68b554e

	fn handle_ebadf<T>(r: io::Result<T>, default: T) -> io::Result<T> {
	match r {
	Err(ref e) if stdio::is_ebadf(e) => Ok(default),
	r => r,
	}
	}

Code using raw handles do not gain this benefit unless they do similar checks themselves.

[sunfishcode]

I've now added a patch to run the child processes directly. The child processes still get NULL handles.

So while there does exist a function inside std named get_handle that returns Err on a NULL handle, in practice, as far as I can tell, every time it's called, the externally visible behavior seems the same as it would be if get_handle returned Ok(NULL) instead. The write would fail with ERROR_INVALID_HANDLE, and the top-level handle_ebadf would see the same result.

[ChrisDenton]

You code doesn't seem to be checking the exit statuses?

In any case I'm uncertain what you're trying to show. I agree that the standard library's internal handling of stdio is great (quibbles about setting null vs invalid aside). Indeed that's my point. It uses proper Rust types and doesn't use sentinel values internally if it can help it. When a foreign interface returns value which may have sentinels, Rust converts near to the boundary.

However, this information is lost with the as_raw_handle. It is up to users to know a) it may not return a handle and b) the two sentinel values. Nothing in the public API suggests this. And intuitions about Unix behaviour would be wrong for Windows.

[sunfishcode]

Ah, you're right, I was forgetting to check the exit statuses. With that fixed I now indeed see that child processes of a parent that lacks a console get a Stdout where as_raw_handle() returns INVALID_HANDLE_VALUE.