Skip to content
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

stdin inherits by default on unix, not on windows #32254

Closed
brson opened this issue Mar 14, 2016 · 6 comments
Closed

stdin inherits by default on unix, not on windows #32254

brson opened this issue Mar 14, 2016 · 6 comments
Labels
O-windows Operating system: Windows

Comments

@brson
Copy link
Contributor

brson commented Mar 14, 2016

In multirust-rs I discovered that the proxy binaries must add command.stdin(process::Stdio::inherit()); to the child commad in order for stdin to work in the subprocess, but only on windows. On unix it works by default.

@brson brson added A-libs O-windows Operating system: Windows labels Mar 14, 2016
@brson brson changed the title stdin inherits be default on unix, not on windows stdin inherits by default on unix, not on windows Mar 14, 2016
@alexcrichton
Copy link
Member

Is this correct? I tested this program and it works fine on Windows/Unix:

use std::env;
use std::io;
use std::process::Command;

fn main() {
    if env::args().len() == 1 {
        let s = Command::new(env::current_exe().unwrap())
                .arg("foo")
                .status()
                .unwrap();
        assert!(s.success());
    } else {
        let mut s = String::new();
        println!("{:?}", io::stdin().read_line(&mut s));
        println!("{}", s);
    }
}

Is there perhaps another test case to try out though?

@rprichard
Copy link
Contributor

That test case is using status(), which according to documentation results in an inherited stdin by default. However, there's this recent commit that changes stdin's default to Stdio::Null for status() and output(). I'm wondering whether that commit breaks a stability guarantee.

I confirmed that, with the current nightly, the test case fails on Linux -- i.e. read_line immediately returns Ok(0).

I then tried changing the call to spawn() instead, but when I did, the read_line() call failed with an I/O error. I ran the test program on Linux from a GUI terminal, so I expected stdin to be a pty. I haven't figured out why that's happening. If I change the parent to invoke head -n3, then head fails with an I/O error.

@brson
Copy link
Contributor Author

brson commented Mar 15, 2016

I am pretty confused now. After going back and checking both Unix and Windows I now have to have process::Stdio::inherit() for both in order for stdin to work in the child (this code is calling .status()). The particular code I'm looking at previously did not specify how to deal with stdin at all. This is on rustc 1.9.0-nightly (c9629d61c 2016-03-10).

@rprichard
Copy link
Contributor

I then tried changing the call to spawn() instead, but when I did, the read_line() call failed with an I/O error. ...

I think this part isn't Rust related; I reproduced it with Python. The parent dies before the child, making the child an orphan, and somehow that interferes with the child's access to the pty.

@brson
Copy link
Contributor Author

brson commented Mar 15, 2016

@alexcrichton Ok, my scenario is actually more complicated.

This is the executable proxy code in multirust-rs. In response to cargo --run the multirust proxy sets inherit on stdin, then runs cargo. cargo runs my executable (in this case another multirust) which then can't read from stdin.

@alexcrichton
Copy link
Member

Ah excellent diagnosis @rprichard! I think this was a bug introduced by #31618, I'll work on a fix.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Mar 15, 2016
This regression was accidentally introduced in rust-lang#31618, and it's just flipping a
boolean!

Closes rust-lang#32254
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 26, 2016
…uron

std: Fix inheriting stdin on status()

This regression was accidentally introduced in rust-lang#31618, and it's just flipping a
boolean!

Closes rust-lang#32254
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 26, 2016
…uron

std: Fix inheriting stdin on status()

This regression was accidentally introduced in rust-lang#31618, and it's just flipping a
boolean!

Closes rust-lang#32254
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows Operating system: Windows
Projects
None yet
Development

No branches or pull requests

3 participants