Windows: std::process::Command may pass asynchronous pipes to child process

[ChrisDenton]

Rust assumes stdio handles are opened for synchronous access (as will most other Windows processes). However, when passing ChildStdOut, ChildStdIn or ChildStdErr to a new Command the pipes passed in are asynchronous. For a full test case see src/test/ui-fulldeps/stdio-from.rs but here's a shorter example:

fn try_it(a: &mut Child, b: &mut Child) -> io::Result<()> {
    let mut child = Command::new("application")
        .stdin(a.stdout.take().unwrap())
        .stdout(b.stdin.take().unwrap())
        .spawn()?;

    assert!(child.wait()?.success());
    Ok(())
}

This is related to an older issue: #38811 (comment)

This can cause soundness issues described in #81357.

[RalfJung]

This has now made #95848 fail 2 times in a row... is there some form of emergency patch that can be applied to resolve this issue?
Cc @rust-lang/infra since this will prevent unrelated PRs from landing.

[klensy]

There is #95841 fix, but it need to be reviewed :-)

[ChrisDenton]

Yeah, I had hoped increasing the pipe buffer size would work well enough to make failures less common until a proper fix was implemented. But unfortunately it's still quite prone to failures.

I'm trying a different reviewer for #95841 in case they're more available right now.

[RalfJung]

@ChrisDenton FWIW, there is still one thing I am confused by: what if someone spawns a program written in Rust using some means other than our own std::process (maybe not even from Rust but from another language). Couldn't they also end up passing async pipes to the Rust process? Would that be a problem? In other words, is "passed-in pipes must be synchronous" now part of the public API of every Rust binary? (That would seem quite bad, if on Windows some programs require to be spawned with sync pipes and some with async pipes. So I might be misunderstanding what happens here.)

[ChrisDenton]

Sure they can spawn their process using asynchronous handles for stdio. They can even do that using std::process if they bring their own handles. But doing this properly would require the cooperation of the child process (e.g. they'd somehow have to just know the pipes are async).

I think it's more correct to say "passed-in pipes must be assumed to be synchronous" is part of the Windows API. And it's what we were always assuming (albeit unintentionally violating in certain circumstances).