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.

[sunfishcode]

If it's important for users to be aware that a Windows program with no console attached has no stdout handle, and to surface this at the earliest origin of the problem, it seems like it isn't enough to make std::io::stdout().try_as_handle() report an error; what we'd really want is to make std::io::stdout() itself report an error. It seems difficult to promote a new mental model, when Rust in other places has chosen to hide these kinds of differences of Windows, in favor of presenting users with Unix-like behavior.

I wonder if it's feasible to change get_handle and/or Stdio::to_handle so that when a parent with a detached console launches a child process, the child process' stdout().as_raw_handle() returns NULL, like it does in the parent. It'd be an observable change, but it's arguably a bug fix. That would fix the overlap aspect of the problem here.

[ChrisDenton]

I wonder if it's feasible to change get_handle and/or Stdio::to_handle so that when a parent with a detached console launches a child process, the child process' stdout().as_raw_handle() returns NULL, like it does in the parent. It'd be an observable change, but it's arguably a bug fix. That would fix the overlap aspect of the problem here.

I would definitely be in favour of this.

what we'd really want is to make std::io::stdout() itself report an error.

Sure. If we were redesigning the high level stdio API that is something I'd be likely to argue for. But I don't think that's on the table.

It seems difficult to promote a new mental model, when Rust in other places has chosen to hide these kinds of differences of Windows, in favor of presenting users with Unix-like behavior.

And to be fair that's perfectly fine for a high level API. It's free to hide the internal implementation details as much as it wants so long as it presents a consistent interface. The trouble is that people want to get at the internals and when they do they carry the high level (or at least POSIX) model with them. Which would be fine if people had to use the Windows API to get it. But Rust itself provides a public lower level API which keeps up the appearance of the POSIX model instead of more correctly modelling the underlying GetStdHandle call. Which feels especially weird because it does (more or less) "the right thing" internally.

I think this is a particular problem for stronger types like BorrowedHandle because usually Rust is very careful with using the type system instead of having special sentinel values. So much so that it recently came up as the first item on a list of "common newbie mistakes". Generally sentinel values from FFI are handled as soon as possible because their meaning can depend on the function being called. This also avoids having lots of defensive tests for valid handles scattered across functions that use handles.

[sunfishcode]

What if we were to say that NULL, at this level of abstraction, is a normal valid handle, and not a sentry. It's just a valid handle to a device that just happens to fail on any I/O operation. It does happen to alias a sentry value used in some Windows APIs, but that's similar to how the process handle aliases the INVALID_HANDLE_VALUE sentry used in other Windows APIs. It's weird, but it's how Windows works.

If the goal is to give users to have a correct mental model of how Windows works, especially at the lower levels, this seems to be the best way. A NULL handle is literally how the Windows APIs work. GetStdHandle reports actual errors by returning INVALID_HANDLE_VALUE, which means it specifically doesn't consider a detached console to be an error. Coercing it into an error gives an incorrect mental model of how the API works.

Consider the code quoted above:

    [...]
    } else if handle.is_null() {
        Err(io::Error::from_raw_os_error(c::ERROR_INVALID_HANDLE as i32))
    } else {
    [...]

This appears to be the reason that Rust has the behavior where child processes of parents with detached consoles see stdout().as_raw_handle() return INVALID_HANDLE_VALUE instead of NULL like in the parent, which we agreed above is a bug. So this is a case where users being aware of NULL and handling it differently led to a bug. A mental model where NULL is just a valid handle would have likely avoided this bug.

This suggests that for the most accurate mental model of how Windows works, to help users avoid a pattern known to have caused a bug, we should present NULL as a valid handle, and not an error.

[sunfishcode]

Ok, I've now done some experiments with this. DuplicateHandle does not work on NULL, though that can be worked around by just special-casing NULL around calls to DuplicateHandle. The bigger problem is that CreateProcessW fails with ERROR_FILE_NOT_FOUND if hStdInput/hStdOutput/hStdErrror are NULL. I don't know of a reasonable way to fix that. So I don't know of a reasonable way to make the value of stdout().as_raw_handle() be NULL in child processes of parents with detached consoles.

[ChrisDenton]

I'm not sure I agree with your reasoning. NULL is a successful result, and expected in some circumstances, but it means there's no handle not that the handle is valid:

If an application does not have associated standard handles, such as a service running on an interactive desktop, and has not redirected them, the return value is NULL.

And indeed NULL is not a valid handle value in any context.

In the standard library, stdio is the special case here. Other types, such as File or JoinHandle, will always produce a valid handle. It seems weird to me to be creating these workarounds when we could simply special case the actual special case. And as a bonus, it forces the user to be aware of this difference between POSIX and Windows.

Example

#![feature(io_safety)]
use std::os::windows::io::{AsHandle, BorrowedHandle, RawHandle};

// A very simplified version of the stdlib type.
struct Stdout;

impl AsHandle for Stdout {
    /// Panics if there is no handle avaliable.
    /// It's recommended that `try_as_handle` is used in case no stdout handle is avaliable.
    fn as_handle(&self) -> BorrowedHandle<'_> {
        self.try_as_handle().expect("no stdout handle")
    }
}

trait TryAsHandle {
    // Alternatively this could return a `Result`.
    fn try_as_handle(&self) -> Option<BorrowedHandle<'_>>;
}
impl TryAsHandle for Stdout {
    /// Returns the handle if one is avaliable, otherwise returns `None`.
    fn try_as_handle(&self) -> Option<BorrowedHandle<'_>> {
        unsafe {
            let result = GetStdHandle(STD_OUTPUT_HANDLE);
            if result.is_null() || result == INVALID_HANDLE_VALUE {
                None
            } else {
                Some(BorrowedHandle::borrow_raw_handle(result))
            }
        }
    }
}

fn main() {
    let stdout = Stdout;

    // Getting the handle should succeed for a console application...
    dbg!(stdout.as_handle());

    // Unset the stdout handle.
    unsafe {
        SetStdHandle(STD_OUTPUT_HANDLE, 0 as _);
    }
    // Prints `None`.
    dbg!(stdout.try_as_handle());
    // Panics.
    dbg!(stdout.as_handle());
}


// Windows API definitions.
type DWORD = u32;
type BOOL = i32;
const STD_OUTPUT_HANDLE: DWORD = -11_i32 as u32;
const INVALID_HANDLE_VALUE: RawHandle = -1_isize as _;
#[link(name="kernel32")]
extern "system" {
    fn GetStdHandle(nStdHandle: DWORD) -> RawHandle;
    fn SetStdHandle(nStdHandle: DWORD, hHandle: RawHandle) -> BOOL;
}

[sunfishcode]

My experiment above to find a way to reconcile stdio handle values in the child with the parent, and my idea above to treat NULL like a valid always-failing handle, were me trying to fit this proposal into a broader scheme. Unfortunately, neither of my idea here turned out to be practical.

Without a broader scheme, and with my own current limited understanding of Windows, and the lack of reports of this problem affecting any users of std, I myself am not confident that this proposal will be a net positive.

[yoshuawuyts]

I've followed up on this question internally at Microsoft, and we would prefer to have a try_as_handle method but preferably have it return Option, not Result.

[sunfishcode]

@yoshuawuyts Thanks for following up! I think we generally agree that some form of try_as_handle would more precisely describe how Windows works here. The next question is whether this precision is worth the downsides of the asymmetries it would introduce, both in the AsHandle vs. AsRawHandle dimension and in the AsHandle and AsFd dimension. These asymmetries will make porting code from the existing Raw traits to their non-Raw counterparts more complex, and make portability between Windows and Unix-family platforms more complex. And since we don't have any reports of real-world problems that try_with_handle would fix, it's difficult to evaluate.

I'd like to propose #93263 as a way to address this issue. It works by making std's API consistent about always presenting absent stdio handles as null, even in child processes of parents with detached consoles.

Compared to the Reasons for try_from_handle listed above, #93263 doesn't surface the error at its origin, and doesn't statically give users the correct mental model. But, it does surface clean errors on any I/O, and it does dynamically give users a reasonable mental model (NULL handle = absent handle, which is the model that Windows' own GetStdHandle itself uses). And, it fixes the problem of overlapping with an actual handle value.

And it retains the Reasons not to do try_from_handle—it keeps AsHandle symmetric with AsRawHandle, and it uses null which will produce a clean error any time it's used and doesn't alias anything.

[sunfishcode]

#93263 has now been merged.