diff --git a/src/libstd/process.rs b/src/libstd/process.rs index ee7cf009fc4d9..d70b9f099037a 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -243,13 +243,15 @@ impl Command { fn spawn_inner(&self, default_io: StdioImp) -> io::Result { 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) ); @@ -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 { - 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 @@ -341,19 +343,30 @@ impl AsInnerMut 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)> + -> io::Result<(imp::Stdio, Option, Option)> { 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), }) } @@ -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 { @@ -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 for Stdio { - fn from_inner(inner: imp::Stdio) -> Stdio { +impl FromInner for Stdio { + fn from_inner(inner: imp::RawStdio) -> Stdio { Stdio(StdioImp::Raw(inner)) } } diff --git a/src/libstd/sys/unix/ext/process.rs b/src/libstd/sys/unix/ext/process.rs index 2d30b016a2d20..cfe7a1f2dda0d 100644 --- a/src/libstd/sys/unix/ext/process.rs +++ b/src/libstd/sys/unix/ext/process.rs @@ -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() diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index 51a85a276eda5..58e205a01cae0 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -283,8 +283,6 @@ impl File { Ok(File(fd)) } - pub fn into_fd(self) -> FileDesc { self.0 } - pub fn file_attr(&self) -> io::Result { let mut stat: raw::stat = unsafe { mem::zeroed() }; try!(cvt(unsafe { diff --git a/src/libstd/sys/unix/pipe.rs b/src/libstd/sys/unix/pipe.rs index 6283a29ae4625..946857c05bc4a 100644 --- a/src/libstd/sys/unix/pipe.rs +++ b/src/libstd/sys/unix/pipe.rs @@ -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 } } diff --git a/src/libstd/sys/unix/process.rs b/src/libstd/sys/unix/process.rs index 76af42d9348de..9178ca7aba4ce 100644 --- a/src/libstd/sys/unix/process.rs +++ b/src/libstd/sys/unix/process.rs @@ -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 @@ -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 { @@ -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 @@ -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) } @@ -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!(), - } - } -} diff --git a/src/libstd/sys/windows/ext/process.rs b/src/libstd/sys/windows/ext/process.rs index 0fd43a450f330..6f59be2687a1b 100644 --- a/src/libstd/sys/windows/ext/process.rs +++ b/src/libstd/sys/windows/ext/process.rs @@ -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 _ diff --git a/src/libstd/sys/windows/pipe.rs b/src/libstd/sys/windows/pipe.rs index b441d8beedbc0..b2a6607314a50 100644 --- a/src/libstd/sys/windows/pipe.rs +++ b/src/libstd/sys/windows/pipe.rs @@ -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 { self.inner.read(buf) } diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index 05073e60f5d26..0b0268d4746ab 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -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}; @@ -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, @@ -356,15 +356,6 @@ fn make_dirp(d: Option<&OsString>) -> (*const u16, Vec) { } 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 { use libc::DUPLICATE_SAME_ACCESS; @@ -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