Skip to content

std: Tweak process raising/lowering implementations #26159

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

Merged
merged 1 commit into from
Jun 11, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 28 additions & 13 deletions src/libstd/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,13 +243,15 @@ impl Command {

fn spawn_inner(&self, default_io: StdioImp) -> io::Result<Child> {
let default_io = Stdio(default_io);
let (their_stdin, our_stdin) = try!(

// See comment on `setup_io` for what `_drop_later` is.
let (their_stdin, our_stdin, _drop_later) = try!(
setup_io(self.stdin.as_ref().unwrap_or(&default_io), true)
);
let (their_stdout, our_stdout) = try!(
let (their_stdout, our_stdout, _drop_later) = try!(
setup_io(self.stdout.as_ref().unwrap_or(&default_io), false)
);
let (their_stderr, our_stderr) = try!(
let (their_stderr, our_stderr, _drop_later) = try!(
setup_io(self.stderr.as_ref().unwrap_or(&default_io), false)
);

Expand All @@ -271,7 +273,7 @@ impl Command {
/// By default, stdin, stdout and stderr are inherited from the parent.
#[stable(feature = "process", since = "1.0.0")]
pub fn spawn(&mut self) -> io::Result<Child> {
self.spawn_inner(StdioImp::Raw(imp::Stdio::Inherit))
self.spawn_inner(StdioImp::Inherit)
}

/// Executes the command as a child process, waiting for it to finish and
Expand Down Expand Up @@ -341,19 +343,30 @@ impl AsInnerMut<imp::Command> for Command {
fn as_inner_mut(&mut self) -> &mut imp::Command { &mut self.inner }
}

// Takes a `Stdio` configuration (this module) and whether the to-be-owned
// handle will be readable.
//
// Returns a triple of (stdio to spawn with, stdio to store, stdio to drop). The
// stdio to spawn with is passed down to the `sys` module and indicates how the
// stdio stream should be set up. The "stdio to store" is an object which
// should be returned in the `Child` that makes its way out. The "stdio to drop"
// represents the raw value of "stdio to spawn with", but is the owned variant
// for it. This needs to be dropped after the child spawns
fn setup_io(io: &Stdio, readable: bool)
-> io::Result<(imp::Stdio, Option<AnonPipe>)>
-> io::Result<(imp::Stdio, Option<AnonPipe>, Option<AnonPipe>)>
{
Ok(match io.0 {
StdioImp::MakePipe => {
let (reader, writer) = try!(pipe::anon_pipe());
if readable {
(imp::Stdio::Piped(reader), Some(writer))
(imp::Stdio::Raw(reader.raw()), Some(writer), Some(reader))
} else {
(imp::Stdio::Piped(writer), Some(reader))
(imp::Stdio::Raw(writer.raw()), Some(reader), Some(writer))
}
}
StdioImp::Raw(ref raw) => (raw.clone_if_copy(), None),
StdioImp::Raw(ref owned) => (imp::Stdio::Raw(owned.raw()), None, None),
StdioImp::Inherit => (imp::Stdio::Inherit, None, None),
StdioImp::None => (imp::Stdio::None, None, None),
})
}

Expand All @@ -379,7 +392,9 @@ pub struct Stdio(StdioImp);
// The internal enum for stdio setup; see below for descriptions.
enum StdioImp {
MakePipe,
Raw(imp::Stdio),
Raw(imp::RawStdio),
Inherit,
None,
}

impl Stdio {
Expand All @@ -389,16 +404,16 @@ impl Stdio {

/// The child inherits from the corresponding parent descriptor.
#[stable(feature = "process", since = "1.0.0")]
pub fn inherit() -> Stdio { Stdio(StdioImp::Raw(imp::Stdio::Inherit)) }
pub fn inherit() -> Stdio { Stdio(StdioImp::Inherit) }

/// This stream will be ignored. This is the equivalent of attaching the
/// stream to `/dev/null`
#[stable(feature = "process", since = "1.0.0")]
pub fn null() -> Stdio { Stdio(StdioImp::Raw(imp::Stdio::None)) }
pub fn null() -> Stdio { Stdio(StdioImp::None) }
}

impl FromInner<imp::Stdio> for Stdio {
fn from_inner(inner: imp::Stdio) -> Stdio {
impl FromInner<imp::RawStdio> for Stdio {
fn from_inner(inner: imp::RawStdio) -> Stdio {
Stdio(StdioImp::Raw(inner))
}
}
Expand Down
28 changes: 5 additions & 23 deletions src/libstd/sys/unix/ext/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,46 +65,28 @@ impl ExitStatusExt for process::ExitStatus {
}
}

#[stable(feature = "from_raw_os", since = "1.1.0")]
#[stable(feature = "process_extensions", since = "1.2.0")]
impl FromRawFd for process::Stdio {
/// Creates a new instance of `Stdio` from the raw underlying file
/// descriptor.
///
/// When this `Stdio` is used as an I/O handle for a child process the given
/// file descriptor will be `dup`d into the destination file descriptor in
/// the child process.
///
/// Note that this function **does not** take ownership of the file
/// descriptor provided and it will **not** be closed when `Stdio` goes out
/// of scope. As a result this method is unsafe because due to the lack of
/// knowledge about the lifetime of the provided file descriptor, this could
/// cause another I/O primitive's ownership property of its file descriptor
/// to be violated.
///
/// Also note that this file descriptor may be used multiple times to spawn
/// processes. For example the `Command::spawn` function could be called
/// more than once to spawn more than one process sharing this file
/// descriptor.
unsafe fn from_raw_fd(fd: RawFd) -> process::Stdio {
process::Stdio::from_inner(sys::process::Stdio::Fd(fd))
process::Stdio::from_inner(sys::fd::FileDesc::new(fd))
}
}

#[stable(feature = "from_raw_os", since = "1.1.0")]
#[stable(feature = "process_extensions", since = "1.2.0")]
impl AsRawFd for process::ChildStdin {
fn as_raw_fd(&self) -> RawFd {
self.as_inner().fd().raw()
}
}

#[stable(feature = "from_raw_os", since = "1.1.0")]
#[stable(feature = "process_extensions", since = "1.2.0")]
impl AsRawFd for process::ChildStdout {
fn as_raw_fd(&self) -> RawFd {
self.as_inner().fd().raw()
}
}

#[stable(feature = "from_raw_os", since = "1.1.0")]
#[stable(feature = "process_extensions", since = "1.2.0")]
impl AsRawFd for process::ChildStderr {
fn as_raw_fd(&self) -> RawFd {
self.as_inner().fd().raw()
Expand Down
2 changes: 0 additions & 2 deletions src/libstd/sys/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,6 @@ impl File {
Ok(File(fd))
}

pub fn into_fd(self) -> FileDesc { self.0 }

pub fn file_attr(&self) -> io::Result<FileAttr> {
let mut stat: raw::stat = unsafe { mem::zeroed() };
try!(cvt(unsafe {
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/sys/unix/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,6 @@ impl AnonPipe {
self.0.write(buf)
}

pub fn raw(&self) -> libc::c_int { self.0.raw() }
pub fn fd(&self) -> &FileDesc { &self.0 }
pub fn into_fd(self) -> FileDesc { self.0 }
}
33 changes: 11 additions & 22 deletions src/libstd/sys/unix/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ use fmt;
use io::{self, Error, ErrorKind};
use libc::{self, pid_t, c_void, c_int, gid_t, uid_t};
use ptr;
use sys::fd::FileDesc;
use sys::fs::{File, OpenOptions};
use sys::pipe::AnonPipe;
use sys::{self, c, cvt, cvt_r};
use sys::fs::{File, OpenOptions};

////////////////////////////////////////////////////////////////////////////////
// Command
Expand Down Expand Up @@ -121,11 +122,12 @@ pub struct Process {

pub enum Stdio {
Inherit,
Piped(AnonPipe),
None,
Fd(c_int),
Raw(c_int),
}

pub type RawStdio = FileDesc;

const CLOEXEC_MSG_FOOTER: &'static [u8] = b"NOEX";

impl Process {
Expand Down Expand Up @@ -252,10 +254,9 @@ impl Process {
}

let setup = |src: Stdio, dst: c_int| {
let fd = match src {
Stdio::Inherit => return true,
Stdio::Fd(fd) => return cvt_r(|| libc::dup2(fd, dst)).is_ok(),
Stdio::Piped(pipe) => pipe.into_fd(),
match src {
Stdio::Inherit => true,
Stdio::Raw(fd) => cvt_r(|| libc::dup2(fd, dst)).is_ok(),

// If a stdio file descriptor is set to be ignored, we open up
// /dev/null into that file descriptor. Otherwise, the first
Expand All @@ -269,13 +270,12 @@ impl Process {
let devnull = CStr::from_ptr(b"/dev/null\0".as_ptr()
as *const _);
if let Ok(f) = File::open_c(devnull, &opts) {
f.into_fd()
cvt_r(|| libc::dup2(f.fd().raw(), dst)).is_ok()
} else {
return false
false
}
}
};
cvt_r(|| libc::dup2(fd.raw(), dst)).is_ok()
}
};

if !setup(in_fd, libc::STDIN_FILENO) { fail(&mut output) }
Expand Down Expand Up @@ -418,14 +418,3 @@ fn translate_status(status: c_int) -> ExitStatus {
ExitStatus::Signal(imp::WTERMSIG(status))
}
}

impl Stdio {
pub fn clone_if_copy(&self) -> Stdio {
match *self {
Stdio::Inherit => Stdio::Inherit,
Stdio::None => Stdio::None,
Stdio::Fd(fd) => Stdio::Fd(fd),
Stdio::Piped(_) => unreachable!(),
}
}
}
31 changes: 8 additions & 23 deletions src/libstd/sys/windows/ext/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,58 +10,43 @@

//! Extensions to `std::process` for Windows.

#![stable(feature = "from_raw_os", since = "1.1.0")]
#![stable(feature = "process_extensions", since = "1.2.0")]

use os::windows::io::{FromRawHandle, RawHandle, AsRawHandle};
use process;
use sys;
use sys_common::{AsInner, FromInner};

#[stable(feature = "from_raw_os", since = "1.1.0")]
#[stable(feature = "process_extensions", since = "1.2.0")]
impl FromRawHandle for process::Stdio {
/// Creates a new instance of `Stdio` from the raw underlying handle.
///
/// When this `Stdio` is used as an I/O handle for a child process the given
/// handle will be duplicated via `DuplicateHandle` to ensure that the
/// handle has the correct permissions to cross the process boundary.
///
/// Note that this function **does not** take ownership of the handle
/// provided and it will **not** be closed when `Stdio` goes out of scope.
/// As a result this method is unsafe because due to the lack of knowledge
/// about the lifetime of the provided handle, this could cause another I/O
/// primitive's ownership property of its handle to be violated.
///
/// Also note that this handle may be used multiple times to spawn
/// processes. For example the `Command::spawn` function could be called
/// more than once to spawn more than one process sharing this handle.
unsafe fn from_raw_handle(handle: RawHandle) -> process::Stdio {
let handle = sys::handle::RawHandle::new(handle as *mut _);
process::Stdio::from_inner(sys::process::Stdio::Handle(handle))
let handle = sys::handle::Handle::new(handle as *mut _);
process::Stdio::from_inner(handle)
}
}

#[stable(feature = "from_raw_os", since = "1.1.0")]
#[stable(feature = "process_extensions", since = "1.2.0")]
impl AsRawHandle for process::Child {
fn as_raw_handle(&self) -> RawHandle {
self.as_inner().handle().raw() as *mut _
}
}

#[stable(feature = "from_raw_os", since = "1.1.0")]
#[stable(feature = "process_extensions", since = "1.2.0")]
impl AsRawHandle for process::ChildStdin {
fn as_raw_handle(&self) -> RawHandle {
self.as_inner().handle().raw() as *mut _
}
}

#[stable(feature = "from_raw_os", since = "1.1.0")]
#[stable(feature = "process_extensions", since = "1.2.0")]
impl AsRawHandle for process::ChildStdout {
fn as_raw_handle(&self) -> RawHandle {
self.as_inner().handle().raw() as *mut _
}
}

#[stable(feature = "from_raw_os", since = "1.1.0")]
#[stable(feature = "process_extensions", since = "1.2.0")]
impl AsRawHandle for process::ChildStderr {
fn as_raw_handle(&self) -> RawHandle {
self.as_inner().handle().raw() as *mut _
Expand Down
2 changes: 2 additions & 0 deletions src/libstd/sys/windows/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
impl AnonPipe {
pub fn handle(&self) -> &Handle { &self.inner }

pub fn raw(&self) -> libc::HANDLE { self.inner.raw() }

pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> {
self.inner.read(buf)
}
Expand Down
22 changes: 5 additions & 17 deletions src/libstd/sys/windows/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ use sync::StaticMutex;
use sys::c;
use sys::fs::{OpenOptions, File};
use sys::handle::{Handle, RawHandle};
use sys::pipe::AnonPipe;
use sys::stdio;
use sys::{self, cvt};
use sys_common::{AsInner, FromInner};
Expand Down Expand Up @@ -107,11 +106,12 @@ pub struct Process {

pub enum Stdio {
Inherit,
Piped(AnonPipe),
None,
Handle(RawHandle),
Raw(libc::HANDLE),
}

pub type RawStdio = Handle;

impl Process {
pub fn spawn(cfg: &Command,
in_handle: Stdio,
Expand Down Expand Up @@ -356,15 +356,6 @@ fn make_dirp(d: Option<&OsString>) -> (*const u16, Vec<u16>) {
}

impl Stdio {
pub fn clone_if_copy(&self) -> Stdio {
match *self {
Stdio::Inherit => Stdio::Inherit,
Stdio::None => Stdio::None,
Stdio::Handle(handle) => Stdio::Handle(handle),
Stdio::Piped(_) => unreachable!(),
}
}

fn to_handle(&self, stdio_id: libc::DWORD) -> io::Result<Handle> {
use libc::DUPLICATE_SAME_ACCESS;

Expand All @@ -374,11 +365,8 @@ impl Stdio {
io.handle().duplicate(0, true, DUPLICATE_SAME_ACCESS)
})
}
Stdio::Handle(ref handle) => {
handle.duplicate(0, true, DUPLICATE_SAME_ACCESS)
}
Stdio::Piped(ref pipe) => {
pipe.handle().duplicate(0, true, DUPLICATE_SAME_ACCESS)
Stdio::Raw(handle) => {
RawHandle::new(handle).duplicate(0, true, DUPLICATE_SAME_ACCESS)
}

// Similarly to unix, we don't actually leave holes for the
Expand Down