Skip to content

Windows: Propagate null handle values when inheriting #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ChrisDenton opened this issue Nov 21, 2022 · 2 comments
Closed

Windows: Propagate null handle values when inheriting #137

ChrisDenton opened this issue Nov 21, 2022 · 2 comments
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@ChrisDenton
Copy link
Member

Proposal

Problem statement

In Windows, stdio handles are (semantically speaking) Option<Handle> where Handle is a non-zero value and None is zero. When spawning a process with Stdio::Inherit, Rust currently turns zero values into -1 values. This has the unfortunate effect of breaking console subprocesses (which typically need stdio) that are spawned from gui applications (that lack stdio by default) because the console process won't be assigned handles from the newly created console (as they usually would in that situation). Worse, -1 is actually a valid handle which means "the current process". So if a console process, for example, waits on stdin and it has a -1 value then the process will end up waiting on itself instead of input.

Motivation, use-cases

Process a.exe:

#![windows_subsystem = "windows"]

fn main() {
    // Run `b.exe`
    // This process has stdio values of `0` because it's using the "windows" subsystem.
    // However, the child process is spawned with `-1` values.
    std::process::Command::new("b.exe").spawn().unwrap();
}

Process b.exe:

fn main() {
    // When starting this process from a "windows" subsystem parent process,
    // there will be no console so one will be created.
    // The usual Windows behaviour is to then replace any unset stdio handles to the new console.
    // (the same is done when manually allocating a console).
    // However, because the parent process set the stdio to a non-zero value, this is suppressed.
    // Which means that this won't print anything.
    println!("hello world!");
}

Solution sketches

These problems can be fixed by properly propagating the zero values instead of converting them to -1. This does mean that there is behaviour change here but I feel they're justified.

Links and related work

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

@ChrisDenton ChrisDenton added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Nov 21, 2022
@Amanieu
Copy link
Member

Amanieu commented Nov 28, 2022

Is there any good reason for Rust to be converting 0 handles to -1 handles? If not then this seems more like a bug than an API change.

@ChrisDenton
Copy link
Member Author

I do personally think it's a bug but it also seems to have been intentional? Although now that I look at it again I'm unsure.

Doing some git archaeology it seems that this was implemented to fix rust-lang/rust#31167. But I think that was the wrong fix. Using INVALID_HANDLE_VALUE makes duplicate_handle "work" (because it is in fact a valid handle value in some contexts) but it doesn't actually make sense. And I do think the original bug was in whatever can't handle null values for stdio handles.

@dtolnay dtolnay added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

3 participants