Auto merge of #30490 - ipetkov:unix-spawn, r=alexcrichton
* If the requested descriptors to inherit are stdio descriptors there are situations where they will not be set correctly * Example: parent's stdout --> child's stderr parent's stderr --> child's stdout * Solution: if the requested descriptors for the child are stdio descriptors, `dup` them before overwriting the child's stdio Example of a program which exhibits the bug: ```rust // stdio.rs use std::io::Write; use std::io::{stdout, stderr}; use std::process::{Command, Stdio}; use std::os::unix::io::FromRawFd; fn main() { stdout().write_all("parent stdout\n".as_bytes()).unwrap(); stderr().write_all("parent stderr\n".as_bytes()).unwrap(); Command::new("sh") .arg("-c") .arg("echo 'child stdout'; echo 'child stderr' 1>&2") .stdin(Stdio::inherit()) .stdout(unsafe { FromRawFd::from_raw_fd(2) }) .stderr(unsafe { FromRawFd::from_raw_fd(1) }) .status() .unwrap_or_else(|e| { panic!("failed to execute process: {}", e) }); } ``` Before: ``` $ rustc --version rustc 1.7.0-nightly (8ad12c3e2
2015-12-19) $ rustc stdio.rs && ./stdio >out 2>err $ cat out parent stdout $ cat err parent stderr child stdout child stderr ``` After (expected): ``` $ rustc --version rustc 1.7.0-dev (712eccee2
2015-12-19) $ rustc stdio.rs && ./stdio >out 2>err $ cat out parent stdout child stderr $ cat err parent stderr child stdout ```
This commit is contained in:
commit
d228cd3964
|
@ -288,6 +288,32 @@ impl Process {
|
|||
unsafe { libc::_exit(1) }
|
||||
}
|
||||
|
||||
// Make sure that the source descriptors are not an stdio descriptor,
|
||||
// otherwise the order which we set the child's descriptors may blow
|
||||
// away a descriptor which we are hoping to save. For example,
|
||||
// suppose we want the child's stderr to be the parent's stdout, and
|
||||
// the child's stdout to be the parent's stderr. No matter which we
|
||||
// dup first, the second will get overwritten prematurely.
|
||||
let maybe_migrate = |src: Stdio, output: &mut AnonPipe| {
|
||||
match src {
|
||||
Stdio::Raw(fd @ libc::STDIN_FILENO) |
|
||||
Stdio::Raw(fd @ libc::STDOUT_FILENO) |
|
||||
Stdio::Raw(fd @ libc::STDERR_FILENO) => {
|
||||
let fd = match cvt_r(|| libc::dup(fd)) {
|
||||
Ok(fd) => fd,
|
||||
Err(_) => fail(output),
|
||||
};
|
||||
let fd = FileDesc::new(fd);
|
||||
fd.set_cloexec();
|
||||
Stdio::Raw(fd.into_raw())
|
||||
},
|
||||
|
||||
s @ Stdio::None |
|
||||
s @ Stdio::Inherit |
|
||||
s @ Stdio::Raw(_) => s,
|
||||
}
|
||||
};
|
||||
|
||||
let setup = |src: Stdio, dst: c_int| {
|
||||
match src {
|
||||
Stdio::Inherit => true,
|
||||
|
@ -313,6 +339,12 @@ impl Process {
|
|||
}
|
||||
};
|
||||
|
||||
// Make sure we migrate all source descriptors before
|
||||
// we start overwriting them
|
||||
let in_fd = maybe_migrate(in_fd, &mut output);
|
||||
let out_fd = maybe_migrate(out_fd, &mut output);
|
||||
let err_fd = maybe_migrate(err_fd, &mut output);
|
||||
|
||||
if !setup(in_fd, libc::STDIN_FILENO) { fail(&mut output) }
|
||||
if !setup(out_fd, libc::STDOUT_FILENO) { fail(&mut output) }
|
||||
if !setup(err_fd, libc::STDERR_FILENO) { fail(&mut output) }
|
||||
|
|
|
@ -0,0 +1,107 @@
|
|||
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
|
||||
// file at the top-level directory of this distribution and at
|
||||
// http://rust-lang.org/COPYRIGHT.
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
|
||||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
|
||||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
|
||||
// option. This file may not be copied, modified, or distributed
|
||||
// except according to those terms.
|
||||
|
||||
// Previously libstd would set stdio descriptors of a child process
|
||||
// by `dup`ing the requested descriptors to inherit directly into the
|
||||
// stdio descriptors. This, however, would incorrectly handle cases
|
||||
// where the descriptors to inherit were already stdio descriptors.
|
||||
// This test checks to avoid that regression.
|
||||
|
||||
#![cfg_attr(unix, feature(libc))]
|
||||
#![cfg_attr(windows, allow(unused_imports))]
|
||||
|
||||
#[cfg(unix)]
|
||||
extern crate libc;
|
||||
|
||||
use std::fs::File;
|
||||
use std::io::{Read, Write};
|
||||
use std::io::{stdout, stderr};
|
||||
use std::process::{Command, Stdio};
|
||||
|
||||
#[cfg(unix)]
|
||||
use std::os::unix::io::FromRawFd;
|
||||
|
||||
#[cfg(not(unix))]
|
||||
fn main() {
|
||||
// Bug not present in Windows
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn main() {
|
||||
let mut args = std::env::args();
|
||||
let name = args.next().unwrap();
|
||||
let args: Vec<String> = args.collect();
|
||||
if let Some("--child") = args.get(0).map(|s| &**s) {
|
||||
return child();
|
||||
} else if !args.is_empty() {
|
||||
panic!("unknown options");
|
||||
}
|
||||
|
||||
let stdout_backup = unsafe { libc::dup(libc::STDOUT_FILENO) };
|
||||
let stderr_backup = unsafe { libc::dup(libc::STDERR_FILENO) };
|
||||
assert!(stdout_backup > -1);
|
||||
assert!(stderr_backup > -1);
|
||||
|
||||
let (stdout_reader, stdout_writer) = pipe();
|
||||
let (stderr_reader, stderr_writer) = pipe();
|
||||
assert!(unsafe { libc::dup2(stdout_writer, libc::STDOUT_FILENO) } > -1);
|
||||
assert!(unsafe { libc::dup2(stderr_writer, libc::STDERR_FILENO) } > -1);
|
||||
|
||||
// Make sure we close any duplicates of the writer end of the pipe,
|
||||
// otherwise we can get stuck reading from the pipe which has open
|
||||
// writers but no one supplying any input
|
||||
assert_eq!(unsafe { libc::close(stdout_writer) }, 0);
|
||||
assert_eq!(unsafe { libc::close(stderr_writer) }, 0);
|
||||
|
||||
stdout().write_all("parent stdout\n".as_bytes()).expect("failed to write to stdout");
|
||||
stderr().write_all("parent stderr\n".as_bytes()).expect("failed to write to stderr");
|
||||
|
||||
let child = {
|
||||
Command::new(name)
|
||||
.arg("--child")
|
||||
.stdin(Stdio::inherit())
|
||||
.stdout(unsafe { FromRawFd::from_raw_fd(libc::STDERR_FILENO) })
|
||||
.stderr(unsafe { FromRawFd::from_raw_fd(libc::STDOUT_FILENO) })
|
||||
.spawn()
|
||||
};
|
||||
|
||||
// The Stdio passed into the Command took over (and closed) std{out, err}
|
||||
// so we should restore them as they were.
|
||||
assert!(unsafe { libc::dup2(stdout_backup, libc::STDOUT_FILENO) } > -1);
|
||||
assert!(unsafe { libc::dup2(stderr_backup, libc::STDERR_FILENO) } > -1);
|
||||
|
||||
// Using File as a shim around the descriptor
|
||||
let mut read = String::new();
|
||||
let mut f: File = unsafe { FromRawFd::from_raw_fd(stdout_reader) };
|
||||
f.read_to_string(&mut read).expect("failed to read from stdout file");
|
||||
assert_eq!(read, "parent stdout\nchild stderr\n");
|
||||
|
||||
// Using File as a shim around the descriptor
|
||||
read.clear();
|
||||
let mut f: File = unsafe { FromRawFd::from_raw_fd(stderr_reader) };
|
||||
f.read_to_string(&mut read).expect("failed to read from stderr file");
|
||||
assert_eq!(read, "parent stderr\nchild stdout\n");
|
||||
|
||||
assert!(child.expect("failed to execute child process").wait().unwrap().success());
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn child() {
|
||||
stdout().write_all("child stdout\n".as_bytes()).expect("child failed to write to stdout");
|
||||
stderr().write_all("child stderr\n".as_bytes()).expect("child failed to write to stderr");
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
/// Returns a pipe (reader, writer combo)
|
||||
fn pipe() -> (i32, i32) {
|
||||
let mut fds = [0; 2];
|
||||
assert_eq!(unsafe { libc::pipe(fds.as_mut_ptr()) }, 0);
|
||||
(fds[0], fds[1])
|
||||
}
|
Loading…
Reference in New Issue