From 6c4198469025bf037f59d617c5b75229546ce68a Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 31 Oct 2015 11:09:43 -0700 Subject: [PATCH 1/9] std: Refactor process spawning on Unix * Build up the argp/envp pointers while the `Command` is being constructed rather than only when `spawn` is called. This will allow better sharing of code between fork/exec paths. * Rename `child_after_fork` to `exec` and have it only perform the exec half of the spawning. This also means the return type has changed to `io::Error` rather than `!` to represent errors that happen. --- src/libstd/process.rs | 4 +- src/libstd/sys/unix/ext/process.rs | 2 +- src/libstd/sys/unix/process.rs | 325 ++++++++++++++--------------- src/libstd/sys/windows/process.rs | 3 - 4 files changed, 166 insertions(+), 168 deletions(-) diff --git a/src/libstd/process.rs b/src/libstd/process.rs index a8da11420d8e2..c64471cc729af 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -209,7 +209,9 @@ impl Command { /// Add multiple arguments to pass to the program. #[stable(feature = "process", since = "1.0.0")] pub fn args>(&mut self, args: &[S]) -> &mut Command { - self.inner.args(args.iter().map(AsRef::as_ref)); + for arg in args { + self.arg(arg.as_ref()); + } self } diff --git a/src/libstd/sys/unix/ext/process.rs b/src/libstd/sys/unix/ext/process.rs index 212aeb0406eba..97938b07f8b95 100644 --- a/src/libstd/sys/unix/ext/process.rs +++ b/src/libstd/sys/unix/ext/process.rs @@ -12,8 +12,8 @@ #![stable(feature = "rust1", since = "1.0.0")] -use os::unix::raw::{uid_t, gid_t}; use os::unix::io::{FromRawFd, RawFd, AsRawFd, IntoRawFd}; +use os::unix::raw::{uid_t, gid_t}; use process; use sys; use sys_common::{AsInnerMut, AsInner, FromInner, IntoInner}; diff --git a/src/libstd/sys/unix/process.rs b/src/libstd/sys/unix/process.rs index f881070d24143..ed512b834f83b 100644 --- a/src/libstd/sys/unix/process.rs +++ b/src/libstd/sys/unix/process.rs @@ -13,27 +13,46 @@ use prelude::v1::*; use os::unix::prelude::*; -use collections::HashMap; +use collections::hash_map::{HashMap, Entry}; use env; use ffi::{OsString, OsStr, CString, CStr}; use fmt; use io::{self, Error, ErrorKind}; -use libc::{self, pid_t, c_void, c_int, gid_t, uid_t}; +use libc::{self, pid_t, c_int, gid_t, uid_t, c_char}; +use mem; use ptr; use sys::fd::FileDesc; use sys::fs::{File, OpenOptions}; -use sys::pipe::AnonPipe; use sys::{self, cvt, cvt_r}; //////////////////////////////////////////////////////////////////////////////// // Command //////////////////////////////////////////////////////////////////////////////// -#[derive(Clone)] pub struct Command { + // Currently we try hard to ensure that the call to `.exec()` doesn't + // actually allocate any memory. While many platforms try to ensure that + // memory allocation works after a fork in a multithreaded process, it's + // been observed to be buggy and somewhat unreliable, so we do our best to + // just not do it at all! + // + // Along those lines, the `argv` and `envp` raw pointers here are exactly + // what's gonna get passed to `execvp`. The `argv` array starts with the + // `program` and ends with a NULL, and the `envp` pointer, if present, is + // also null-terminated. + // + // Right now we don't support removing arguments, so there's no much fancy + // support there, but we support adding and removing environment variables, + // so a side table is used to track where in the `envp` array each key is + // located. Whenever we add a key we update it in place if it's already + // present, and whenever we remove a key we update the locations of all + // other keys. program: CString, args: Vec, - env: Option>, // Guaranteed to have no NULs. + env: Option>, + argv: Vec<*const c_char>, + envp: Option>, + cwd: Option, uid: Option, gid: Option, @@ -44,10 +63,13 @@ pub struct Command { impl Command { pub fn new(program: &OsStr) -> Command { let mut saw_nul = false; + let program = os2c(program, &mut saw_nul); Command { - program: os2c(program, &mut saw_nul), + argv: vec![program.as_ptr(), 0 as *const _], + program: program, args: Vec::new(), env: None, + envp: None, cwd: None, uid: None, gid: None, @@ -57,40 +79,79 @@ impl Command { } pub fn arg(&mut self, arg: &OsStr) { - self.args.push(os2c(arg, &mut self.saw_nul)); + // Overwrite the trailing NULL pointer in `argv` and then add a new null + // pointer. + let arg = os2c(arg, &mut self.saw_nul); + self.argv[self.args.len() + 1] = arg.as_ptr(); + self.argv.push(0 as *const _); + + // Also make sure we keep track of the owned value to schedule a + // destructor for this memory. + self.args.push(arg); } - pub fn args<'a, I: Iterator>(&mut self, args: I) { - let mut saw_nul = self.saw_nul; - self.args.extend(args.map(|arg| os2c(arg, &mut saw_nul))); - self.saw_nul = saw_nul; - } - fn init_env_map(&mut self) { + + fn init_env_map(&mut self) -> (&mut HashMap, + &mut Vec<*const c_char>) { if self.env.is_none() { - // Will not add NULs to env: preexisting environment will not contain any. - self.env = Some(env::vars_os().collect()); + let mut map = HashMap::new(); + let mut envp = Vec::new(); + for (k, v) in env::vars_os() { + let s = pair_to_key(&k, &v, &mut self.saw_nul); + envp.push(s.as_ptr()); + map.insert(k, (envp.len() - 1, s)); + } + envp.push(0 as *const _); + self.env = Some(map); + self.envp = Some(envp); } + (self.env.as_mut().unwrap(), self.envp.as_mut().unwrap()) } - pub fn env(&mut self, key: &OsStr, val: &OsStr) { - let k = OsString::from_vec(os2c(key, &mut self.saw_nul).into_bytes()); - let v = OsString::from_vec(os2c(val, &mut self.saw_nul).into_bytes()); - // Will not add NULs to env: return without inserting if any were seen. - if self.saw_nul { - return; + pub fn env(&mut self, key: &OsStr, val: &OsStr) { + let new_key = pair_to_key(key, val, &mut self.saw_nul); + let (map, envp) = self.init_env_map(); + + // If `key` is already present then we we just update `envp` in place + // (and store the owned value), but if it's not there we override the + // trailing NULL pointer, add a new NULL pointer, and store where we + // were located. + match map.entry(key.to_owned()) { + Entry::Occupied(mut e) => { + let (i, ref mut s) = *e.get_mut(); + envp[i] = new_key.as_ptr(); + *s = new_key; + } + Entry::Vacant(e) => { + let len = envp.len(); + envp[len - 1] = new_key.as_ptr(); + envp.push(0 as *const _); + e.insert((len - 1, new_key)); + } } - - self.init_env_map(); - self.env.as_mut() - .unwrap() - .insert(k, v); } + pub fn env_remove(&mut self, key: &OsStr) { - self.init_env_map(); - self.env.as_mut().unwrap().remove(key); + let (map, envp) = self.init_env_map(); + + // If we actually ended up removing a key, then we need to update the + // position of all keys that come after us in `envp` because they're all + // one element sooner now. + if let Some((i, _)) = map.remove(key) { + envp.remove(i); + + for (_, &mut (ref mut j, _)) in map.iter_mut() { + if *j >= i { + *j -= 1; + } + } + } } + pub fn env_clear(&mut self) { - self.env = Some(HashMap::new()) + self.env = Some(HashMap::new()); + self.envp = Some(vec![0 as *const _]); } + pub fn cwd(&mut self, dir: &OsStr) { self.cwd = Some(os2c(dir, &mut self.saw_nul)); } @@ -112,6 +173,18 @@ fn os2c(s: &OsStr, saw_nul: &mut bool) -> CString { }) } +fn pair_to_key(key: &OsStr, value: &OsStr, saw_nul: &mut bool) -> CString { + let (key, value) = (key.as_bytes(), value.as_bytes()); + let mut v = Vec::with_capacity(key.len() + value.len() + 1); + v.extend(key); + v.push(b'='); + v.extend(value); + CString::new(v).unwrap_or_else(|_e| { + *saw_nul = true; + CString::new("foo=bar").unwrap() + }) +} + impl fmt::Debug for Command { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { try!(write!(f, "{:?}", self.program)); @@ -218,20 +291,28 @@ impl Process { return Err(io::Error::new(ErrorKind::InvalidInput, "nul byte found in provided data")); } - let dirp = cfg.cwd.as_ref().map(|c| c.as_ptr()).unwrap_or(ptr::null()); - - let (envp, _a, _b) = make_envp(cfg.env.as_ref()); - let (argv, _a) = make_argv(&cfg.program, &cfg.args); let (input, output) = try!(sys::pipe::anon_pipe()); let pid = unsafe { - match libc::fork() { + match try!(cvt(libc::fork())) { 0 => { drop(input); - Process::child_after_fork(cfg, output, argv, envp, dirp, - in_fd, out_fd, err_fd) + let err = Process::exec(cfg, in_fd, out_fd, err_fd); + let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32; + let bytes = [ + (errno >> 24) as u8, + (errno >> 16) as u8, + (errno >> 8) as u8, + (errno >> 0) as u8, + CLOEXEC_MSG_FOOTER[0], CLOEXEC_MSG_FOOTER[1], + CLOEXEC_MSG_FOOTER[2], CLOEXEC_MSG_FOOTER[3] + ]; + // pipe I/O up to PIPE_BUF bytes should be atomic, and then + // we want to be sure we *don't* run at_exit destructors as + // we're being torn down regardless + assert!(output.write(&bytes).is_ok()); + libc::_exit(1) } - n if n < 0 => return Err(Error::last_os_error()), n => n, } }; @@ -306,29 +387,15 @@ impl Process { // allocation). Instead we just close it manually. This will never // have the drop glue anyway because this code never returns (the // child will either exec() or invoke libc::exit) - unsafe fn child_after_fork(cfg: &Command, - mut output: AnonPipe, - argv: *const *const libc::c_char, - envp: *const libc::c_void, - dirp: *const libc::c_char, - in_fd: Stdio, - out_fd: Stdio, - err_fd: Stdio) -> ! { - fn fail(output: &mut AnonPipe) -> ! { - let errno = sys::os::errno() as u32; - let bytes = [ - (errno >> 24) as u8, - (errno >> 16) as u8, - (errno >> 8) as u8, - (errno >> 0) as u8, - CLOEXEC_MSG_FOOTER[0], CLOEXEC_MSG_FOOTER[1], - CLOEXEC_MSG_FOOTER[2], CLOEXEC_MSG_FOOTER[3] - ]; - // pipe I/O up to PIPE_BUF bytes should be atomic, and then we want - // to be sure we *don't* run at_exit destructors as we're being torn - // down regardless - assert!(output.write(&bytes).is_ok()); - unsafe { libc::_exit(1) } + unsafe fn exec(cfg: &Command, + in_fd: Stdio, + out_fd: Stdio, + err_fd: Stdio) -> io::Error { + macro_rules! try { + ($e:expr) => (match $e { + Ok(e) => e, + Err(e) => return e, + }) } // Make sure that the source descriptors are not an stdio descriptor, @@ -337,30 +404,30 @@ impl Process { // 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| { + let maybe_migrate = |src: Stdio| { 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()) - }, - + cvt_r(|| libc::dup(fd)).map(|fd| { + let fd = FileDesc::new(fd); + fd.set_cloexec(); + Stdio::Raw(fd.into_raw()) + }) + } s @ Stdio::None | s @ Stdio::Inherit | - s @ Stdio::Raw(_) => s, + s @ Stdio::Raw(_) => Ok(s), } }; + let in_fd = try!(maybe_migrate(in_fd)); + let out_fd = try!(maybe_migrate(out_fd)); + let err_fd = try!(maybe_migrate(err_fd)); let setup = |src: Stdio, dst: c_int| { match src { - Stdio::Inherit => true, - Stdio::Raw(fd) => cvt_r(|| libc::dup2(fd, dst)).is_ok(), + Stdio::Inherit => Ok(()), + Stdio::Raw(fd) => cvt_r(|| libc::dup2(fd, dst)).map(|_| ()), // If a stdio file descriptor is set to be ignored, we open up // /dev/null into that file descriptor. Otherwise, the first @@ -373,29 +440,18 @@ impl Process { opts.write(dst != libc::STDIN_FILENO); let devnull = CStr::from_ptr(b"/dev/null\0".as_ptr() as *const _); - if let Ok(f) = File::open_c(devnull, &opts) { - cvt_r(|| libc::dup2(f.fd().raw(), dst)).is_ok() - } else { - false - } + File::open_c(devnull, &opts).and_then(|f| { + cvt_r(|| libc::dup2(f.fd().raw(), dst)).map(|_| ()) + }) } } }; - - // 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) } + try!(setup(in_fd, libc::STDIN_FILENO)); + try!(setup(out_fd, libc::STDOUT_FILENO)); + try!(setup(err_fd, libc::STDERR_FILENO)); if let Some(u) = cfg.gid { - if libc::setgid(u as libc::gid_t) != 0 { - fail(&mut output); - } + try!(cvt(libc::setgid(u as gid_t))); } if let Some(u) = cfg.uid { // When dropping privileges from root, the `setgroups` call @@ -407,9 +463,7 @@ impl Process { // privilege dropping function. let _ = libc::setgroups(0, ptr::null()); - if libc::setuid(u as libc::uid_t) != 0 { - fail(&mut output); - } + try!(cvt(libc::setuid(u as uid_t))); } if cfg.session_leader { // Don't check the error of setsid because it fails if we're the @@ -417,16 +471,15 @@ impl Process { // error, but ignore it anyway. let _ = libc::setsid(); } - if !dirp.is_null() && libc::chdir(dirp) == -1 { - fail(&mut output); + if let Some(ref cwd) = cfg.cwd { + try!(cvt(libc::chdir(cwd.as_ptr()))); } - if !envp.is_null() { - *sys::os::environ() = envp as *const _; + if let Some(ref envp) = cfg.envp { + *sys::os::environ() = envp.as_ptr(); } - #[cfg(not(target_os = "nacl"))] - unsafe fn reset_signal_handling(output: &mut AnonPipe) { - use mem; + // NaCl has no signal support. + if cfg!(not(target_os = "nacl")) { // Reset signal handling so the child process starts in a // standardized state. libstd ignores SIGPIPE, and signal-handling // libraries often set a mask. Child processes inherit ignored @@ -435,23 +488,17 @@ impl Process { // need to clean things up now to avoid confusing the program // we're about to run. let mut set: libc::sigset_t = mem::uninitialized(); - if libc::sigemptyset(&mut set) != 0 || - libc::pthread_sigmask(libc::SIG_SETMASK, &set, ptr::null_mut()) != 0 || - libc::signal( - libc::SIGPIPE, mem::transmute(libc::SIG_DFL) - ) == mem::transmute(libc::SIG_ERR) - { - fail(output); + try!(cvt(libc::sigemptyset(&mut set))); + try!(cvt(libc::pthread_sigmask(libc::SIG_SETMASK, &set, + ptr::null_mut()))); + let ret = libc::signal(libc::SIGPIPE, libc::SIG_DFL); + if ret == libc::SIG_ERR { + return io::Error::last_os_error() } } - #[cfg(target_os = "nacl")] - unsafe fn reset_signal_handling(_output: &mut AnonPipe) { - // NaCl has no signal support. - } - reset_signal_handling(&mut output); - let _ = libc::execvp(*argv, argv); - fail(&mut output) + libc::execvp(cfg.argv[0], cfg.argv.as_ptr()); + io::Error::last_os_error() } pub fn id(&self) -> u32 { @@ -477,54 +524,6 @@ impl Process { } } -fn make_argv(prog: &CString, args: &[CString]) - -> (*const *const libc::c_char, Vec<*const libc::c_char>) -{ - let mut ptrs: Vec<*const libc::c_char> = Vec::with_capacity(args.len()+1); - - // Convert the CStrings into an array of pointers. Also return the - // vector that owns the raw pointers, to ensure they live long - // enough. - ptrs.push(prog.as_ptr()); - ptrs.extend(args.iter().map(|tmp| tmp.as_ptr())); - - // Add a terminating null pointer (required by libc). - ptrs.push(ptr::null()); - - (ptrs.as_ptr(), ptrs) -} - -fn make_envp(env: Option<&HashMap>) - -> (*const c_void, Vec>, Vec<*const libc::c_char>) -{ - // On posixy systems we can pass a char** for envp, which is a - // null-terminated array of "k=v\0" strings. As with make_argv, we - // return two vectors that own the data to ensure that they live - // long enough. - if let Some(env) = env { - let mut tmps = Vec::with_capacity(env.len()); - - for pair in env { - let mut kv = Vec::new(); - kv.extend_from_slice(pair.0.as_bytes()); - kv.push('=' as u8); - kv.extend_from_slice(pair.1.as_bytes()); - kv.push(0); // terminating null - tmps.push(kv); - } - - let mut ptrs: Vec<*const libc::c_char> = - tmps.iter() - .map(|tmp| tmp.as_ptr() as *const libc::c_char) - .collect(); - ptrs.push(ptr::null()); - - (ptrs.as_ptr() as *const _, tmps, ptrs) - } else { - (ptr::null(), Vec::new(), Vec::new()) - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index 61cf28be16f91..6a04aa2f2c4f3 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -74,9 +74,6 @@ impl Command { pub fn arg(&mut self, arg: &OsStr) { self.args.push(arg.to_os_string()) } - pub fn args<'a, I: Iterator>(&mut self, args: I) { - self.args.extend(args.map(OsStr::to_os_string)) - } fn init_env_map(&mut self){ if self.env.is_none() { self.env = Some(env::vars_os().map(|(key, val)| { From b1898db0f10f9641c7616e93499348d4fe743ddd Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 3 Feb 2016 16:55:59 -0800 Subject: [PATCH 2/9] std: Implement CommandExt::before_exec This is a Unix-specific function which adds the ability to register a closure to run pre-exec to configure the child process as required (note that these closures are run post-fork). cc #31398 --- src/libstd/process.rs | 4 +- src/libstd/sys/unix/ext/process.rs | 38 ++++++++++ src/libstd/sys/unix/process.rs | 15 +++- src/test/run-pass/command-before-exec.rs | 90 ++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 4 deletions(-) create mode 100644 src/test/run-pass/command-before-exec.rs diff --git a/src/libstd/process.rs b/src/libstd/process.rs index c64471cc729af..e11bb72a35a58 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -269,7 +269,7 @@ impl Command { self } - fn spawn_inner(&self, default_io: StdioImp) -> io::Result { + fn spawn_inner(&mut self, default_io: StdioImp) -> io::Result { let default_io = Stdio(default_io); // See comment on `setup_io` for what `_drop_later` is. @@ -283,7 +283,7 @@ impl Command { setup_io(self.stderr.as_ref().unwrap_or(&default_io), false) ); - match imp::Process::spawn(&self.inner, their_stdin, their_stdout, + match imp::Process::spawn(&mut self.inner, their_stdin, their_stdout, their_stderr) { Err(e) => Err(e), Ok(handle) => Ok(Child { diff --git a/src/libstd/sys/unix/ext/process.rs b/src/libstd/sys/unix/ext/process.rs index 97938b07f8b95..96727ed66745a 100644 --- a/src/libstd/sys/unix/ext/process.rs +++ b/src/libstd/sys/unix/ext/process.rs @@ -12,6 +12,9 @@ #![stable(feature = "rust1", since = "1.0.0")] +use prelude::v1::*; + +use io; use os::unix::io::{FromRawFd, RawFd, AsRawFd, IntoRawFd}; use os::unix::raw::{uid_t, gid_t}; use process; @@ -44,6 +47,34 @@ pub trait CommandExt { #[unstable(feature = "process_session_leader", reason = "recently added", issue = "27811")] fn session_leader(&mut self, on: bool) -> &mut process::Command; + + /// Schedules a closure to be run just before the `exec` function is + /// invoked. + /// + /// The closure is allowed to return an I/O error whose OS error code will + /// be communicated back to the parent and returned as an error from when + /// the spawn was requested. + /// + /// Multiple closures can be registered and they will be called in order of + /// their registration. If a closure returns `Err` then no further closures + /// will be called and the spawn operation will immediately return with a + /// failure. + /// + /// # Notes + /// + /// This closure will be run in the context of the child process after a + /// `fork`. This primarily means that any modificatons made to memory on + /// behalf of this closure will **not** be visible to the parent process. + /// This is often a very constrained environment where normal operations + /// like `malloc` or acquiring a mutex are not guaranteed to work (due to + /// other threads perhaps still running when the `fork` was run). + /// + /// When this closure is run, aspects such as the stdio file descriptors and + /// working directory have successfully been changed, so output to these + /// locations may not appear where intended. + #[unstable(feature = "process_exec", issue = "31398")] + fn before_exec(&mut self, f: F) -> &mut process::Command + where F: FnMut() -> io::Result<()> + Send + Sync + 'static; } #[stable(feature = "rust1", since = "1.0.0")] @@ -62,6 +93,13 @@ impl CommandExt for process::Command { self.as_inner_mut().session_leader(on); self } + + fn before_exec(&mut self, f: F) -> &mut process::Command + where F: FnMut() -> io::Result<()> + Send + Sync + 'static + { + self.as_inner_mut().before_exec(Box::new(f)); + self + } } /// Unix-specific extensions to `std::process::ExitStatus` diff --git a/src/libstd/sys/unix/process.rs b/src/libstd/sys/unix/process.rs index ed512b834f83b..7387e9def9f04 100644 --- a/src/libstd/sys/unix/process.rs +++ b/src/libstd/sys/unix/process.rs @@ -58,6 +58,7 @@ pub struct Command { gid: Option, session_leader: bool, saw_nul: bool, + closures: Vec io::Result<()> + Send + Sync>>, } impl Command { @@ -75,6 +76,7 @@ impl Command { gid: None, session_leader: false, saw_nul: saw_nul, + closures: Vec::new(), } } @@ -164,6 +166,11 @@ impl Command { pub fn session_leader(&mut self, session_leader: bool) { self.session_leader = session_leader; } + + pub fn before_exec(&mut self, + f: Box io::Result<()> + Send + Sync>) { + self.closures.push(f); + } } fn os2c(s: &OsStr, saw_nul: &mut bool) -> CString { @@ -283,7 +290,7 @@ impl Process { Ok(()) } - pub fn spawn(cfg: &Command, + pub fn spawn(cfg: &mut Command, in_fd: Stdio, out_fd: Stdio, err_fd: Stdio) -> io::Result { @@ -387,7 +394,7 @@ impl Process { // allocation). Instead we just close it manually. This will never // have the drop glue anyway because this code never returns (the // child will either exec() or invoke libc::exit) - unsafe fn exec(cfg: &Command, + unsafe fn exec(cfg: &mut Command, in_fd: Stdio, out_fd: Stdio, err_fd: Stdio) -> io::Error { @@ -497,6 +504,10 @@ impl Process { } } + for callback in cfg.closures.iter_mut() { + try!(callback()); + } + libc::execvp(cfg.argv[0], cfg.argv.as_ptr()); io::Error::last_os_error() } diff --git a/src/test/run-pass/command-before-exec.rs b/src/test/run-pass/command-before-exec.rs new file mode 100644 index 0000000000000..16560637b6926 --- /dev/null +++ b/src/test/run-pass/command-before-exec.rs @@ -0,0 +1,90 @@ +// Copyright 2016 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// ignore-windows - this is a unix-specific test + +#![feature(process_exec, libc)] + +extern crate libc; + +use std::env; +use std::io::Error; +use std::os::unix::process::CommandExt; +use std::process::Command; +use std::sync::Arc; +use std::sync::atomic::{AtomicUsize, Ordering}; + +fn main() { + if let Some(arg) = env::args().skip(1).next() { + match &arg[..] { + "test1" => println!("hello2"), + "test2" => assert_eq!(env::var("FOO").unwrap(), "BAR"), + "test3" => assert_eq!(env::current_dir().unwrap() + .to_str().unwrap(), "/"), + "empty" => {} + _ => panic!("unknown argument: {}", arg), + } + return + } + + let me = env::current_exe().unwrap(); + + let output = Command::new(&me).arg("test1").before_exec(|| { + println!("hello"); + Ok(()) + }).output().unwrap(); + assert!(output.status.success()); + assert!(output.stderr.is_empty()); + assert_eq!(output.stdout, b"hello\nhello2\n"); + + let output = Command::new(&me).arg("test2").before_exec(|| { + env::set_var("FOO", "BAR"); + Ok(()) + }).output().unwrap(); + assert!(output.status.success()); + assert!(output.stderr.is_empty()); + assert!(output.stdout.is_empty()); + + let output = Command::new(&me).arg("test3").before_exec(|| { + env::set_current_dir("/").unwrap(); + Ok(()) + }).output().unwrap(); + assert!(output.status.success()); + assert!(output.stderr.is_empty()); + assert!(output.stdout.is_empty()); + + let output = Command::new(&me).arg("bad").before_exec(|| { + Err(Error::from_raw_os_error(102)) + }).output().err().unwrap(); + assert_eq!(output.raw_os_error(), Some(102)); + + let pid = unsafe { libc::getpid() }; + assert!(pid >= 0); + let output = Command::new(&me).arg("empty").before_exec(move || { + let child = unsafe { libc::getpid() }; + assert!(child >= 0); + assert!(pid != child); + Ok(()) + }).output().unwrap(); + assert!(output.status.success()); + assert!(output.stderr.is_empty()); + assert!(output.stdout.is_empty()); + + let mem = Arc::new(AtomicUsize::new(0)); + let mem2 = mem.clone(); + let output = Command::new(&me).arg("empty").before_exec(move || { + assert_eq!(mem2.fetch_add(1, Ordering::SeqCst), 0); + Ok(()) + }).output().unwrap(); + assert!(output.status.success()); + assert!(output.stderr.is_empty()); + assert!(output.stdout.is_empty()); + assert_eq!(mem.load(Ordering::SeqCst), 0); +} From 627515a7ff4fe12084d7e95969bda307849b4d0e Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 3 Feb 2016 18:09:35 -0800 Subject: [PATCH 3/9] std: Push Child's exit status to sys::process On Unix we have to be careful to not call `waitpid` twice, but we don't have to be careful on Windows due to the way process handles work there. As a result the cached `Option` is only necessary on Unix, and it's also just an implementation detail of the Unix module. At the same time. also update some code in `kill` on Unix to avoid a wonky waitpid with WNOHANG. This was added in 0e190b9a to solve #13124, but the `signal(0)` method is not supported any more so there's no need to for this workaround. I believe that this is no longer necessary as it's not really doing anything. --- src/libstd/process.rs | 49 ++++--------------------------- src/libstd/sys/unix/process.rs | 42 +++++++++++++------------- src/libstd/sys/windows/process.rs | 8 +++-- 3 files changed, 30 insertions(+), 69 deletions(-) diff --git a/src/libstd/process.rs b/src/libstd/process.rs index e11bb72a35a58..819643f20fe81 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -11,15 +11,14 @@ //! Working with processes. #![stable(feature = "process", since = "1.0.0")] -#![allow(non_upper_case_globals)] use prelude::v1::*; use io::prelude::*; use ffi::OsStr; use fmt; -use io::{self, Error, ErrorKind}; -use path; +use io; +use path::Path; use str; use sys::pipe::{self, AnonPipe}; use sys::process as imp; @@ -61,9 +60,6 @@ use thread::{self, JoinHandle}; pub struct Child { handle: imp::Process, - /// None until wait() or wait_with_output() is called. - status: Option, - /// The handle for writing to the child's stdin, if it has been captured #[stable(feature = "process", since = "1.0.0")] pub stdin: Option, @@ -243,7 +239,7 @@ impl Command { /// Sets the working directory for the child process. #[stable(feature = "process", since = "1.0.0")] - pub fn current_dir>(&mut self, dir: P) -> &mut Command { + pub fn current_dir>(&mut self, dir: P) -> &mut Command { self.inner.cwd(dir.as_ref().as_ref()); self } @@ -288,7 +284,6 @@ impl Command { Err(e) => Err(e), Ok(handle) => Ok(Child { handle: handle, - status: None, stdin: our_stdin.map(|fd| ChildStdin { inner: fd }), stdout: our_stdout.map(|fd| ChildStdout { inner: fd }), stderr: our_stderr.map(|fd| ChildStderr { inner: fd }), @@ -508,34 +503,7 @@ impl Child { /// SIGKILL on unix platforms. #[stable(feature = "process", since = "1.0.0")] pub fn kill(&mut self) -> io::Result<()> { - #[cfg(unix)] fn collect_status(p: &mut Child) { - // On Linux (and possibly other unices), a process that has exited will - // continue to accept signals because it is "defunct". The delivery of - // signals will only fail once the child has been reaped. For this - // reason, if the process hasn't exited yet, then we attempt to collect - // their status with WNOHANG. - if p.status.is_none() { - match p.handle.try_wait() { - Some(status) => { p.status = Some(status); } - None => {} - } - } - } - #[cfg(windows)] fn collect_status(_p: &mut Child) {} - - collect_status(self); - - // if the process has finished, and therefore had waitpid called, - // and we kill it, then on unix we might ending up killing a - // newer process that happens to have the same (re-used) id - if self.status.is_some() { - return Err(Error::new( - ErrorKind::InvalidInput, - "invalid argument: can't kill an exited process", - )) - } - - unsafe { self.handle.kill() } + self.handle.kill() } /// Returns the OS-assigned process identifier associated with this child. @@ -555,14 +523,7 @@ impl Child { #[stable(feature = "process", since = "1.0.0")] pub fn wait(&mut self) -> io::Result { drop(self.stdin.take()); - match self.status { - Some(code) => Ok(ExitStatus(code)), - None => { - let status = try!(self.handle.wait()); - self.status = Some(status); - Ok(ExitStatus(status)) - } - } + self.handle.wait().map(ExitStatus) } /// Simultaneously waits for the child to exit and collect all remaining diff --git a/src/libstd/sys/unix/process.rs b/src/libstd/sys/unix/process.rs index 7387e9def9f04..41b9b3ef12632 100644 --- a/src/libstd/sys/unix/process.rs +++ b/src/libstd/sys/unix/process.rs @@ -8,8 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![allow(non_snake_case)] - use prelude::v1::*; use os::unix::prelude::*; @@ -271,7 +269,8 @@ impl fmt::Display for ExitStatus { /// The unique id of the process (this should never be negative). pub struct Process { - pid: pid_t + pid: pid_t, + status: Option, } pub enum Stdio { @@ -285,11 +284,6 @@ pub type RawStdio = FileDesc; const CLOEXEC_MSG_FOOTER: &'static [u8] = b"NOEX"; impl Process { - pub unsafe fn kill(&self) -> io::Result<()> { - try!(cvt(libc::kill(self.pid, libc::SIGKILL))); - Ok(()) - } - pub fn spawn(cfg: &mut Command, in_fd: Stdio, out_fd: Stdio, @@ -324,7 +318,7 @@ impl Process { } }; - let p = Process{ pid: pid }; + let mut p = Process { pid: pid, status: None }; drop(output); let mut bytes = [0; 8]; @@ -516,22 +510,26 @@ impl Process { self.pid as u32 } - pub fn wait(&self) -> io::Result { - let mut status = 0 as c_int; - try!(cvt_r(|| unsafe { libc::waitpid(self.pid, &mut status, 0) })); - Ok(ExitStatus(status)) + pub fn kill(&mut self) -> io::Result<()> { + // If we've already waited on this process then the pid can be recycled + // and used for another process, and we probably shouldn't be killing + // random processes, so just return an error. + if self.status.is_some() { + Err(Error::new(ErrorKind::InvalidInput, + "invalid argument: can't kill an exited process")) + } else { + cvt(unsafe { libc::kill(self.pid, libc::SIGKILL) }).map(|_| ()) + } } - pub fn try_wait(&self) -> Option { - let mut status = 0 as c_int; - match cvt_r(|| unsafe { - libc::waitpid(self.pid, &mut status, libc::WNOHANG) - }) { - Ok(0) => None, - Ok(n) if n == self.pid => Some(ExitStatus(status)), - Ok(n) => panic!("unknown pid: {}", n), - Err(e) => panic!("unknown waitpid error: {}", e), + pub fn wait(&mut self) -> io::Result { + if let Some(status) = self.status { + return Ok(status) } + let mut status = 0 as c_int; + try!(cvt_r(|| unsafe { libc::waitpid(self.pid, &mut status, 0) })); + self.status = Some(ExitStatus(status)); + Ok(ExitStatus(status)) } } diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index 6a04aa2f2c4f3..e5e4187d228e9 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -202,8 +202,10 @@ impl Process { Ok(Process { handle: Handle::new(pi.hProcess) }) } - pub unsafe fn kill(&self) -> io::Result<()> { - try!(cvt(c::TerminateProcess(self.handle.raw(), 1))); + pub fn kill(&mut self) -> io::Result<()> { + try!(cvt(unsafe { + c::TerminateProcess(self.handle.raw(), 1) + })); Ok(()) } @@ -213,7 +215,7 @@ impl Process { } } - pub fn wait(&self) -> io::Result { + pub fn wait(&mut self) -> io::Result { unsafe { let res = c::WaitForSingleObject(self.handle.raw(), c::INFINITE); if res != c::WAIT_OBJECT_0 { From b8bd8f3d7c9c8a3187d6c80ab201f66dedee457c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 4 Feb 2016 09:53:01 -0800 Subject: [PATCH 4/9] std: Rename Stdio::None to Stdio::Null This better reflects what it's actually doing as we don't actually have an option for "leave this I/O slot as an empty hole". --- src/libstd/process.rs | 6 +++--- src/libstd/sys/unix/process.rs | 16 +++++++--------- src/libstd/sys/windows/process.rs | 11 +++++------ 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/libstd/process.rs b/src/libstd/process.rs index 819643f20fe81..fb217f9da0400 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -385,7 +385,7 @@ fn setup_io(io: &Stdio, readable: bool) } 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), + StdioImp::Null => (imp::Stdio::Null, None, None), }) } @@ -439,7 +439,7 @@ enum StdioImp { MakePipe, Raw(imp::RawStdio), Inherit, - None, + Null, } impl Stdio { @@ -454,7 +454,7 @@ impl Stdio { /// 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::None) } + pub fn null() -> Stdio { Stdio(StdioImp::Null) } } impl FromInner for Stdio { diff --git a/src/libstd/sys/unix/process.rs b/src/libstd/sys/unix/process.rs index 41b9b3ef12632..f776af296163d 100644 --- a/src/libstd/sys/unix/process.rs +++ b/src/libstd/sys/unix/process.rs @@ -275,7 +275,7 @@ pub struct Process { pub enum Stdio { Inherit, - None, + Null, Raw(c_int), } @@ -416,7 +416,7 @@ impl Process { Stdio::Raw(fd.into_raw()) }) } - s @ Stdio::None | + s @ Stdio::Null | s @ Stdio::Inherit | s @ Stdio::Raw(_) => Ok(s), } @@ -430,12 +430,10 @@ impl Process { Stdio::Inherit => Ok(()), Stdio::Raw(fd) => cvt_r(|| libc::dup2(fd, dst)).map(|_| ()), - // If a stdio file descriptor is set to be ignored, we open up - // /dev/null into that file descriptor. Otherwise, the first - // file descriptor opened up in the child would be numbered as - // one of the stdio file descriptors, which is likely to wreak - // havoc. - Stdio::None => { + // Open up a reference to /dev/null with appropriate read/write + // permissions and then move it into the correct location via + // `dup2`. + Stdio::Null => { let mut opts = OpenOptions::new(); opts.read(dst == libc::STDIN_FILENO); opts.write(dst != libc::STDIN_FILENO); @@ -590,7 +588,7 @@ mod tests { let cat = t!(Process::spawn(&cmd, Stdio::Raw(stdin_read.raw()), Stdio::Raw(stdout_write.raw()), - Stdio::None)); + Stdio::Null)); drop(stdin_read); drop(stdout_write); diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index e5e4187d228e9..8a522a0a7952a 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -122,7 +122,7 @@ pub struct Process { pub enum Stdio { Inherit, - None, + Null, Raw(c::HANDLE), } @@ -386,11 +386,10 @@ impl Stdio { RawHandle::new(handle).duplicate(0, true, c::DUPLICATE_SAME_ACCESS) } - // Similarly to unix, we don't actually leave holes for the - // stdio file descriptors, but rather open up /dev/null - // equivalents. These equivalents are drawn from libuv's - // windows process spawning. - Stdio::None => { + // Open up a reference to NUL with appropriate read/write + // permissions as well as the ability to be inherited to child + // processes (as this is about to be inherited). + Stdio::Null => { let size = mem::size_of::(); let mut sa = c::SECURITY_ATTRIBUTES { nLength: size as c::DWORD, From 18f9a79c23da8e8920e4c944656b9945f3544337 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 4 Feb 2016 09:59:47 -0800 Subject: [PATCH 5/9] std: Lift out Windows' CreateProcess lock a bit The function `CreateProcess` is not itself unsafe to call from many threads, the article in question is pointing out that handles can be inherited by unintended child processes. This is basically the same race as the standard Unix open-then-set-cloexec race. Since the intention of the lock is to protect children from inheriting unintended handles, the lock is now lifted out to before the creation of the child I/O handles (which will all be inheritable). This will ensure that we only have one process in Rust at least creating inheritable handles at a time, preventing unintended inheritance to children. --- src/libstd/sys/windows/process.rs | 33 +++++++++++++++++++------------ 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index 8a522a0a7952a..2f548a708039c 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -159,14 +159,6 @@ impl Process { si.cb = mem::size_of::() as c::DWORD; si.dwFlags = c::STARTF_USESTDHANDLES; - let stdin = try!(in_handle.to_handle(c::STD_INPUT_HANDLE)); - let stdout = try!(out_handle.to_handle(c::STD_OUTPUT_HANDLE)); - let stderr = try!(err_handle.to_handle(c::STD_ERROR_HANDLE)); - - si.hStdInput = stdin.raw(); - si.hStdOutput = stdout.raw(); - si.hStdError = stderr.raw(); - let program = program.as_ref().unwrap_or(&cfg.program); let mut cmd_str = try!(make_command_line(program, &cfg.args)); cmd_str.push(0); // add null terminator @@ -180,12 +172,27 @@ impl Process { let (envp, _data) = try!(make_envp(cfg.env.as_ref())); let (dirp, _data) = try!(make_dirp(cfg.cwd.as_ref())); let mut pi = zeroed_process_information(); - try!(unsafe { - // `CreateProcess` is racy! - // http://support.microsoft.com/kb/315939 - static CREATE_PROCESS_LOCK: StaticMutex = StaticMutex::new(); - let _lock = CREATE_PROCESS_LOCK.lock(); + // Prepare all stdio handles to be inherited by the child. This + // currently involves duplicating any existing ones with the ability to + // be inherited by child processes. Note, however, that once an + // inheritable handle is created, *any* spawned child will inherit that + // handle. We only want our own child to inherit this handle, so we wrap + // the remaining portion of this spawn in a mutex. + // + // For more information, msdn also has an article about this race: + // http://support.microsoft.com/kb/315939 + static CREATE_PROCESS_LOCK: StaticMutex = StaticMutex::new(); + let _lock = CREATE_PROCESS_LOCK.lock(); + + let stdin = try!(in_handle.to_handle(c::STD_INPUT_HANDLE)); + let stdout = try!(out_handle.to_handle(c::STD_OUTPUT_HANDLE)); + let stderr = try!(err_handle.to_handle(c::STD_ERROR_HANDLE)); + si.hStdInput = stdin.raw(); + si.hStdOutput = stdout.raw(); + si.hStdError = stderr.raw(); + + try!(unsafe { cvt(c::CreateProcessW(ptr::null(), cmd_str.as_mut_ptr(), ptr::null_mut(), From d15db1d392c9126ed5cc766753f08540c08a3626 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 4 Feb 2016 11:10:37 -0800 Subject: [PATCH 6/9] std: Push process stdio setup in std::sys Most of this is platform-specific anyway, and we generally have to jump through fewer hoops to do the equivalent operation on Windows. One benefit for Windows today is that this new structure avoids an extra `DuplicateHandle` when creating pipes. For Unix, however, the behavior should be the same. Note that this is just a pure refactoring, no functionality was added or removed. --- src/libstd/process.rs | 129 +++----- src/libstd/sys/unix/ext/process.rs | 4 +- src/libstd/sys/unix/pipe.rs | 1 - src/libstd/sys/unix/process.rs | 443 +++++++++++++++----------- src/libstd/sys/windows/ext/process.rs | 3 +- src/libstd/sys/windows/pipe.rs | 2 - src/libstd/sys/windows/process.rs | 225 +++++++------ 7 files changed, 440 insertions(+), 367 deletions(-) diff --git a/src/libstd/process.rs b/src/libstd/process.rs index fb217f9da0400..8db8ad324bea9 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -20,7 +20,7 @@ use fmt; use io; use path::Path; use str; -use sys::pipe::{self, AnonPipe}; +use sys::pipe::AnonPipe; use sys::process as imp; use sys_common::{AsInner, AsInnerMut, FromInner, IntoInner}; use thread::{self, JoinHandle}; @@ -77,6 +77,17 @@ impl AsInner for Child { fn as_inner(&self) -> &imp::Process { &self.handle } } +impl FromInner<(imp::Process, imp::StdioPipes)> for Child { + fn from_inner((handle, io): (imp::Process, imp::StdioPipes)) -> Child { + Child { + handle: handle, + stdin: io.stdin.map(ChildStdin::from_inner), + stdout: io.stdout.map(ChildStdout::from_inner), + stderr: io.stderr.map(ChildStderr::from_inner), + } + } +} + impl IntoInner for Child { fn into_inner(self) -> imp::Process { self.handle } } @@ -106,6 +117,12 @@ impl IntoInner for ChildStdin { fn into_inner(self) -> AnonPipe { self.inner } } +impl FromInner for ChildStdin { + fn from_inner(pipe: AnonPipe) -> ChildStdin { + ChildStdin { inner: pipe } + } +} + /// A handle to a child process's stdout #[stable(feature = "process", since = "1.0.0")] pub struct ChildStdout { @@ -127,6 +144,12 @@ impl IntoInner for ChildStdout { fn into_inner(self) -> AnonPipe { self.inner } } +impl FromInner for ChildStdout { + fn from_inner(pipe: AnonPipe) -> ChildStdout { + ChildStdout { inner: pipe } + } +} + /// A handle to a child process's stderr #[stable(feature = "process", since = "1.0.0")] pub struct ChildStderr { @@ -148,6 +171,12 @@ impl IntoInner for ChildStderr { fn into_inner(self) -> AnonPipe { self.inner } } +impl FromInner for ChildStderr { + fn from_inner(pipe: AnonPipe) -> ChildStderr { + ChildStderr { inner: pipe } + } +} + /// The `Command` type acts as a process builder, providing fine-grained control /// over how a new process should be spawned. A default configuration can be /// generated using `Command::new(program)`, where `program` gives a path to the @@ -167,11 +196,6 @@ impl IntoInner for ChildStderr { #[stable(feature = "process", since = "1.0.0")] pub struct Command { inner: imp::Command, - - // Details explained in the builder methods - stdin: Option, - stdout: Option, - stderr: Option, } impl Command { @@ -187,12 +211,7 @@ impl Command { /// otherwise configure the process. #[stable(feature = "process", since = "1.0.0")] pub fn new>(program: S) -> Command { - Command { - inner: imp::Command::new(program.as_ref()), - stdin: None, - stdout: None, - stderr: None, - } + Command { inner: imp::Command::new(program.as_ref()) } } /// Add an argument to pass to the program. @@ -247,56 +266,30 @@ impl Command { /// Configuration for the child process's stdin handle (file descriptor 0). #[stable(feature = "process", since = "1.0.0")] pub fn stdin(&mut self, cfg: Stdio) -> &mut Command { - self.stdin = Some(cfg); + self.inner.stdin(cfg.0); self } /// Configuration for the child process's stdout handle (file descriptor 1). #[stable(feature = "process", since = "1.0.0")] pub fn stdout(&mut self, cfg: Stdio) -> &mut Command { - self.stdout = Some(cfg); + self.inner.stdout(cfg.0); self } /// Configuration for the child process's stderr handle (file descriptor 2). #[stable(feature = "process", since = "1.0.0")] pub fn stderr(&mut self, cfg: Stdio) -> &mut Command { - self.stderr = Some(cfg); + self.inner.stderr(cfg.0); self } - fn spawn_inner(&mut self, default_io: StdioImp) -> io::Result { - let default_io = Stdio(default_io); - - // 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, _drop_later) = try!( - setup_io(self.stdout.as_ref().unwrap_or(&default_io), false) - ); - let (their_stderr, our_stderr, _drop_later) = try!( - setup_io(self.stderr.as_ref().unwrap_or(&default_io), false) - ); - - match imp::Process::spawn(&mut self.inner, their_stdin, their_stdout, - their_stderr) { - Err(e) => Err(e), - Ok(handle) => Ok(Child { - handle: handle, - stdin: our_stdin.map(|fd| ChildStdin { inner: fd }), - stdout: our_stdout.map(|fd| ChildStdout { inner: fd }), - stderr: our_stderr.map(|fd| ChildStderr { inner: fd }), - }) - } - } - /// Executes the command as a child process, returning a handle to it. /// /// 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::Inherit) + self.inner.spawn(imp::Stdio::Inherit).map(Child::from_inner) } /// Executes the command as a child process, waiting for it to finish and @@ -319,7 +312,8 @@ impl Command { /// ``` #[stable(feature = "process", since = "1.0.0")] pub fn output(&mut self) -> io::Result { - self.spawn_inner(StdioImp::MakePipe).and_then(|p| p.wait_with_output()) + self.inner.spawn(imp::Stdio::MakePipe).map(Child::from_inner) + .and_then(|p| p.wait_with_output()) } /// Executes a command as a child process, waiting for it to finish and @@ -362,33 +356,6 @@ 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, Option)> -{ - Ok(match io.0 { - StdioImp::MakePipe => { - let (reader, writer) = try!(pipe::anon_pipe()); - if readable { - (imp::Stdio::Raw(reader.raw()), Some(writer), Some(reader)) - } else { - (imp::Stdio::Raw(writer.raw()), Some(reader), Some(writer)) - } - } - StdioImp::Raw(ref owned) => (imp::Stdio::Raw(owned.raw()), None, None), - StdioImp::Inherit => (imp::Stdio::Inherit, None, None), - StdioImp::Null => (imp::Stdio::Null, None, None), - }) -} - /// The output of a finished process. #[derive(PartialEq, Eq, Clone)] #[stable(feature = "process", since = "1.0.0")] @@ -432,34 +399,26 @@ impl fmt::Debug for Output { /// Describes what to do with a standard I/O stream for a child process. #[stable(feature = "process", since = "1.0.0")] -pub struct Stdio(StdioImp); - -// The internal enum for stdio setup; see below for descriptions. -enum StdioImp { - MakePipe, - Raw(imp::RawStdio), - Inherit, - Null, -} +pub struct Stdio(imp::Stdio); impl Stdio { /// A new pipe should be arranged to connect the parent and child processes. #[stable(feature = "process", since = "1.0.0")] - pub fn piped() -> Stdio { Stdio(StdioImp::MakePipe) } + pub fn piped() -> Stdio { Stdio(imp::Stdio::MakePipe) } /// The child inherits from the corresponding parent descriptor. #[stable(feature = "process", since = "1.0.0")] - pub fn inherit() -> Stdio { Stdio(StdioImp::Inherit) } + pub fn inherit() -> Stdio { Stdio(imp::Stdio::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::Null) } + pub fn null() -> Stdio { Stdio(imp::Stdio::Null) } } -impl FromInner for Stdio { - fn from_inner(inner: imp::RawStdio) -> Stdio { - Stdio(StdioImp::Raw(inner)) +impl FromInner for Stdio { + fn from_inner(inner: imp::Stdio) -> Stdio { + Stdio(inner) } } diff --git a/src/libstd/sys/unix/ext/process.rs b/src/libstd/sys/unix/ext/process.rs index 96727ed66745a..cf72cfd7e507e 100644 --- a/src/libstd/sys/unix/ext/process.rs +++ b/src/libstd/sys/unix/ext/process.rs @@ -120,7 +120,9 @@ impl ExitStatusExt for process::ExitStatus { #[stable(feature = "process_extensions", since = "1.2.0")] impl FromRawFd for process::Stdio { unsafe fn from_raw_fd(fd: RawFd) -> process::Stdio { - process::Stdio::from_inner(sys::fd::FileDesc::new(fd)) + let fd = sys::fd::FileDesc::new(fd); + let io = sys::process::Stdio::Fd(fd); + process::Stdio::from_inner(io) } } diff --git a/src/libstd/sys/unix/pipe.rs b/src/libstd/sys/unix/pipe.rs index 9527b1e2243d3..667f0f9e6bf62 100644 --- a/src/libstd/sys/unix/pipe.rs +++ b/src/libstd/sys/unix/pipe.rs @@ -61,7 +61,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 f776af296163d..4716d25c0b28f 100644 --- a/src/libstd/sys/unix/process.rs +++ b/src/libstd/sys/unix/process.rs @@ -21,6 +21,7 @@ use mem; use ptr; use sys::fd::FileDesc; use sys::fs::{File, OpenOptions}; +use sys::pipe::{self, AnonPipe}; use sys::{self, cvt, cvt_r}; //////////////////////////////////////////////////////////////////////////////// @@ -57,6 +58,38 @@ pub struct Command { session_leader: bool, saw_nul: bool, closures: Vec io::Result<()> + Send + Sync>>, + stdin: Option, + stdout: Option, + stderr: Option, +} + +// passed back to std::process with the pipes connected to the child, if any +// were requested +pub struct StdioPipes { + pub stdin: Option, + pub stdout: Option, + pub stderr: Option, +} + +// passed to do_exec() with configuration of what the child stdio should look +// like +struct ChildPipes { + stdin: ChildStdio, + stdout: ChildStdio, + stderr: ChildStdio, +} + +enum ChildStdio { + Inherit, + Explicit(c_int), + Owned(FileDesc), +} + +pub enum Stdio { + Inherit, + Null, + MakePipe, + Fd(FileDesc), } impl Command { @@ -75,6 +108,9 @@ impl Command { session_leader: false, saw_nul: saw_nul, closures: Vec::new(), + stdin: None, + stdout: None, + stderr: None, } } @@ -169,136 +205,32 @@ impl Command { f: Box io::Result<()> + Send + Sync>) { self.closures.push(f); } -} - -fn os2c(s: &OsStr, saw_nul: &mut bool) -> CString { - CString::new(s.as_bytes()).unwrap_or_else(|_e| { - *saw_nul = true; - CString::new("").unwrap() - }) -} - -fn pair_to_key(key: &OsStr, value: &OsStr, saw_nul: &mut bool) -> CString { - let (key, value) = (key.as_bytes(), value.as_bytes()); - let mut v = Vec::with_capacity(key.len() + value.len() + 1); - v.extend(key); - v.push(b'='); - v.extend(value); - CString::new(v).unwrap_or_else(|_e| { - *saw_nul = true; - CString::new("foo=bar").unwrap() - }) -} - -impl fmt::Debug for Command { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - try!(write!(f, "{:?}", self.program)); - for arg in &self.args { - try!(write!(f, " {:?}", arg)); - } - Ok(()) - } -} - -//////////////////////////////////////////////////////////////////////////////// -// Processes -//////////////////////////////////////////////////////////////////////////////// - -/// Unix exit statuses -#[derive(PartialEq, Eq, Clone, Copy, Debug)] -pub struct ExitStatus(c_int); - -#[cfg(any(target_os = "linux", target_os = "android", - target_os = "nacl", target_os = "solaris", - target_os = "emscripten"))] -mod status_imp { - pub fn WIFEXITED(status: i32) -> bool { (status & 0xff) == 0 } - pub fn WEXITSTATUS(status: i32) -> i32 { (status >> 8) & 0xff } - pub fn WTERMSIG(status: i32) -> i32 { status & 0x7f } -} - -#[cfg(any(target_os = "macos", - target_os = "ios", - target_os = "freebsd", - target_os = "dragonfly", - target_os = "bitrig", - target_os = "netbsd", - target_os = "openbsd"))] -mod status_imp { - pub fn WIFEXITED(status: i32) -> bool { (status & 0x7f) == 0 } - pub fn WEXITSTATUS(status: i32) -> i32 { status >> 8 } - pub fn WTERMSIG(status: i32) -> i32 { status & 0o177 } -} - -impl ExitStatus { - fn exited(&self) -> bool { - status_imp::WIFEXITED(self.0) - } - pub fn success(&self) -> bool { - self.code() == Some(0) + pub fn stdin(&mut self, stdin: Stdio) { + self.stdin = Some(stdin); } - - pub fn code(&self) -> Option { - if self.exited() { - Some(status_imp::WEXITSTATUS(self.0)) - } else { - None - } + pub fn stdout(&mut self, stdout: Stdio) { + self.stdout = Some(stdout); } - - pub fn signal(&self) -> Option { - if !self.exited() { - Some(status_imp::WTERMSIG(self.0)) - } else { - None - } + pub fn stderr(&mut self, stderr: Stdio) { + self.stderr = Some(stderr); } -} -impl fmt::Display for ExitStatus { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - if let Some(code) = self.code() { - write!(f, "exit code: {}", code) - } else { - let signal = self.signal().unwrap(); - write!(f, "signal: {}", signal) - } - } -} - -/// The unique id of the process (this should never be negative). -pub struct Process { - pid: pid_t, - status: Option, -} - -pub enum Stdio { - Inherit, - Null, - Raw(c_int), -} - -pub type RawStdio = FileDesc; - -const CLOEXEC_MSG_FOOTER: &'static [u8] = b"NOEX"; - -impl Process { - pub fn spawn(cfg: &mut Command, - in_fd: Stdio, - out_fd: Stdio, - err_fd: Stdio) -> io::Result { - if cfg.saw_nul { - return Err(io::Error::new(ErrorKind::InvalidInput, "nul byte found in provided data")); + pub fn spawn(&mut self, default: Stdio) + -> io::Result<(Process, StdioPipes)> { + if self.saw_nul { + return Err(io::Error::new(ErrorKind::InvalidInput, + "nul byte found in provided data")); } + let (ours, theirs) = try!(self.setup_io(default)); let (input, output) = try!(sys::pipe::anon_pipe()); let pid = unsafe { match try!(cvt(libc::fork())) { 0 => { drop(input); - let err = Process::exec(cfg, in_fd, out_fd, err_fd); + let err = self.exec(theirs); let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32; let bytes = [ (errno >> 24) as u8, @@ -325,7 +257,7 @@ impl Process { // loop to handle EINTR loop { match input.read(&mut bytes) { - Ok(0) => return Ok(p), + Ok(0) => return Ok((p, ours)), Ok(8) => { assert!(combine(CLOEXEC_MSG_FOOTER) == combine(&bytes[4.. 8]), "Validation on the CLOEXEC pipe failed: {:?}", bytes); @@ -388,10 +320,7 @@ impl Process { // allocation). Instead we just close it manually. This will never // have the drop glue anyway because this code never returns (the // child will either exec() or invoke libc::exit) - unsafe fn exec(cfg: &mut Command, - in_fd: Stdio, - out_fd: Stdio, - err_fd: Stdio) -> io::Error { + unsafe fn exec(&mut self, stdio: ChildPipes) -> io::Error { macro_rules! try { ($e:expr) => (match $e { Ok(e) => e, @@ -399,60 +328,20 @@ impl Process { }) } - // 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| { - match src { - Stdio::Raw(fd @ libc::STDIN_FILENO) | - Stdio::Raw(fd @ libc::STDOUT_FILENO) | - Stdio::Raw(fd @ libc::STDERR_FILENO) => { - cvt_r(|| libc::dup(fd)).map(|fd| { - let fd = FileDesc::new(fd); - fd.set_cloexec(); - Stdio::Raw(fd.into_raw()) - }) - } - s @ Stdio::Null | - s @ Stdio::Inherit | - s @ Stdio::Raw(_) => Ok(s), - } - }; - let in_fd = try!(maybe_migrate(in_fd)); - let out_fd = try!(maybe_migrate(out_fd)); - let err_fd = try!(maybe_migrate(err_fd)); - - let setup = |src: Stdio, dst: c_int| { - match src { - Stdio::Inherit => Ok(()), - Stdio::Raw(fd) => cvt_r(|| libc::dup2(fd, dst)).map(|_| ()), - - // Open up a reference to /dev/null with appropriate read/write - // permissions and then move it into the correct location via - // `dup2`. - Stdio::Null => { - let mut opts = OpenOptions::new(); - opts.read(dst == libc::STDIN_FILENO); - opts.write(dst != libc::STDIN_FILENO); - let devnull = CStr::from_ptr(b"/dev/null\0".as_ptr() - as *const _); - File::open_c(devnull, &opts).and_then(|f| { - cvt_r(|| libc::dup2(f.fd().raw(), dst)).map(|_| ()) - }) - } - } - }; - try!(setup(in_fd, libc::STDIN_FILENO)); - try!(setup(out_fd, libc::STDOUT_FILENO)); - try!(setup(err_fd, libc::STDERR_FILENO)); + if let Some(fd) = stdio.stdin.fd() { + try!(cvt_r(|| libc::dup2(fd, libc::STDIN_FILENO))); + } + if let Some(fd) = stdio.stdout.fd() { + try!(cvt_r(|| libc::dup2(fd, libc::STDOUT_FILENO))); + } + if let Some(fd) = stdio.stderr.fd() { + try!(cvt_r(|| libc::dup2(fd, libc::STDERR_FILENO))); + } - if let Some(u) = cfg.gid { + if let Some(u) = self.gid { try!(cvt(libc::setgid(u as gid_t))); } - if let Some(u) = cfg.uid { + if let Some(u) = self.uid { // When dropping privileges from root, the `setgroups` call // will remove any extraneous groups. If we don't call this, // then even though our uid has dropped, we may still have @@ -464,16 +353,16 @@ impl Process { try!(cvt(libc::setuid(u as uid_t))); } - if cfg.session_leader { + if self.session_leader { // Don't check the error of setsid because it fails if we're the // process leader already. We just forked so it shouldn't return // error, but ignore it anyway. let _ = libc::setsid(); } - if let Some(ref cwd) = cfg.cwd { + if let Some(ref cwd) = self.cwd { try!(cvt(libc::chdir(cwd.as_ptr()))); } - if let Some(ref envp) = cfg.envp { + if let Some(ref envp) = self.envp { *sys::os::environ() = envp.as_ptr(); } @@ -496,14 +385,195 @@ impl Process { } } - for callback in cfg.closures.iter_mut() { + for callback in self.closures.iter_mut() { try!(callback()); } - libc::execvp(cfg.argv[0], cfg.argv.as_ptr()); + libc::execvp(self.argv[0], self.argv.as_ptr()); io::Error::last_os_error() } + + fn setup_io(&self, default: Stdio) -> io::Result<(StdioPipes, ChildPipes)> { + let stdin = self.stdin.as_ref().unwrap_or(&default); + let stdout = self.stdout.as_ref().unwrap_or(&default); + let stderr = self.stderr.as_ref().unwrap_or(&default); + let (their_stdin, our_stdin) = try!(stdin.to_child_stdio(true)); + let (their_stdout, our_stdout) = try!(stdout.to_child_stdio(false)); + let (their_stderr, our_stderr) = try!(stderr.to_child_stdio(false)); + let ours = StdioPipes { + stdin: our_stdin, + stdout: our_stdout, + stderr: our_stderr, + }; + let theirs = ChildPipes { + stdin: their_stdin, + stdout: their_stdout, + stderr: their_stderr, + }; + Ok((ours, theirs)) + } +} + +fn os2c(s: &OsStr, saw_nul: &mut bool) -> CString { + CString::new(s.as_bytes()).unwrap_or_else(|_e| { + *saw_nul = true; + CString::new("").unwrap() + }) +} + +impl Stdio { + fn to_child_stdio(&self, readable: bool) + -> io::Result<(ChildStdio, Option)> { + match *self { + Stdio::Inherit => Ok((ChildStdio::Inherit, None)), + + // 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. + Stdio::Fd(ref fd) => { + if fd.raw() >= 0 && fd.raw() <= libc::STDERR_FILENO { + Ok((ChildStdio::Owned(try!(fd.duplicate())), None)) + } else { + Ok((ChildStdio::Explicit(fd.raw()), None)) + } + } + + Stdio::MakePipe => { + let (reader, writer) = try!(pipe::anon_pipe()); + let (ours, theirs) = if readable { + (writer, reader) + } else { + (reader, writer) + }; + Ok((ChildStdio::Owned(theirs.into_fd()), Some(ours))) + } + + Stdio::Null => { + let mut opts = OpenOptions::new(); + opts.read(readable); + opts.write(!readable); + let path = unsafe { + CStr::from_ptr("/dev/null\0".as_ptr() as *const _) + }; + let fd = try!(File::open_c(&path, &opts)); + Ok((ChildStdio::Owned(fd.into_fd()), None)) + } + } + } +} + +impl ChildStdio { + fn fd(&self) -> Option { + match *self { + ChildStdio::Inherit => None, + ChildStdio::Explicit(fd) => Some(fd), + ChildStdio::Owned(ref fd) => Some(fd.raw()), + } + } +} + +fn pair_to_key(key: &OsStr, value: &OsStr, saw_nul: &mut bool) -> CString { + let (key, value) = (key.as_bytes(), value.as_bytes()); + let mut v = Vec::with_capacity(key.len() + value.len() + 1); + v.extend(key); + v.push(b'='); + v.extend(value); + CString::new(v).unwrap_or_else(|_e| { + *saw_nul = true; + CString::new("foo=bar").unwrap() + }) +} + +impl fmt::Debug for Command { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + try!(write!(f, "{:?}", self.program)); + for arg in &self.args { + try!(write!(f, " {:?}", arg)); + } + Ok(()) + } +} + +//////////////////////////////////////////////////////////////////////////////// +// Processes +//////////////////////////////////////////////////////////////////////////////// + +/// Unix exit statuses +#[derive(PartialEq, Eq, Clone, Copy, Debug)] +pub struct ExitStatus(c_int); + +#[cfg(any(target_os = "linux", target_os = "android", + target_os = "nacl"))] +mod status_imp { + pub fn WIFEXITED(status: i32) -> bool { (status & 0xff) == 0 } + pub fn WEXITSTATUS(status: i32) -> i32 { (status >> 8) & 0xff } + pub fn WTERMSIG(status: i32) -> i32 { status & 0x7f } +} + +#[cfg(any(target_os = "macos", + target_os = "ios", + target_os = "freebsd", + target_os = "dragonfly", + target_os = "bitrig", + target_os = "netbsd", + target_os = "openbsd"))] +mod status_imp { + pub fn WIFEXITED(status: i32) -> bool { (status & 0x7f) == 0 } + pub fn WEXITSTATUS(status: i32) -> i32 { status >> 8 } + pub fn WTERMSIG(status: i32) -> i32 { status & 0o177 } +} + +impl ExitStatus { + fn exited(&self) -> bool { + status_imp::WIFEXITED(self.0) + } + + pub fn success(&self) -> bool { + self.code() == Some(0) + } + + pub fn code(&self) -> Option { + if self.exited() { + Some(status_imp::WEXITSTATUS(self.0)) + } else { + None + } + } + + pub fn signal(&self) -> Option { + if !self.exited() { + Some(status_imp::WTERMSIG(self.0)) + } else { + None + } + } +} + +impl fmt::Display for ExitStatus { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + if let Some(code) = self.code() { + write!(f, "exit code: {}", code) + } else { + let signal = self.signal().unwrap(); + write!(f, "signal: {}", signal) + } + } +} + +/// The unique id of the process (this should never be negative). +pub struct Process { + pid: pid_t, + status: Option, +} + +const CLOEXEC_MSG_FOOTER: &'static [u8] = b"NOEX"; + +impl Process { pub fn id(&self) -> u32 { self.pid as u32 } @@ -540,7 +610,7 @@ mod tests { use mem; use ptr; use libc; - use sys::{self, cvt}; + use sys::cvt; macro_rules! t { ($e:expr) => { @@ -576,9 +646,7 @@ mod tests { fn test_process_mask() { unsafe { // Test to make sure that a signal mask does not get inherited. - let cmd = Command::new(OsStr::new("cat")); - let (stdin_read, stdin_write) = t!(sys::pipe::anon_pipe()); - let (stdout_read, stdout_write) = t!(sys::pipe::anon_pipe()); + let mut cmd = Command::new(OsStr::new("cat")); let mut set: libc::sigset_t = mem::uninitialized(); let mut old_set: libc::sigset_t = mem::uninitialized(); @@ -586,11 +654,12 @@ mod tests { t!(cvt(sigaddset(&mut set, libc::SIGINT))); t!(cvt(libc::pthread_sigmask(libc::SIG_SETMASK, &set, &mut old_set))); - let cat = t!(Process::spawn(&cmd, Stdio::Raw(stdin_read.raw()), - Stdio::Raw(stdout_write.raw()), - Stdio::Null)); - drop(stdin_read); - drop(stdout_write); + cmd.stdin(Stdio::MakePipe); + cmd.stdout(Stdio::MakePipe); + + let (mut cat, mut pipes) = t!(cmd.spawn(Stdio::Null)); + let stdin_write = pipes.stdin.take().unwrap(); + let stdout_read = pipes.stdout.take().unwrap(); t!(cvt(libc::pthread_sigmask(libc::SIG_SETMASK, &old_set, ptr::null_mut()))); diff --git a/src/libstd/sys/windows/ext/process.rs b/src/libstd/sys/windows/ext/process.rs index dffe68915fb45..f6ee59eec327a 100644 --- a/src/libstd/sys/windows/ext/process.rs +++ b/src/libstd/sys/windows/ext/process.rs @@ -21,7 +21,8 @@ use sys_common::{AsInner, FromInner, IntoInner}; impl FromRawHandle for process::Stdio { unsafe fn from_raw_handle(handle: RawHandle) -> process::Stdio { let handle = sys::handle::Handle::new(handle as *mut _); - process::Stdio::from_inner(handle) + let io = sys::process::Stdio::Handle(handle); + process::Stdio::from_inner(io) } } diff --git a/src/libstd/sys/windows/pipe.rs b/src/libstd/sys/windows/pipe.rs index a6e69c789d098..aec41885f3b87 100644 --- a/src/libstd/sys/windows/pipe.rs +++ b/src/libstd/sys/windows/pipe.rs @@ -37,8 +37,6 @@ impl AnonPipe { pub fn handle(&self) -> &Handle { &self.inner } pub fn into_handle(self) -> Handle { self.inner } - pub fn raw(&self) -> c::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 2f548a708039c..fa118be6fe6b1 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -26,9 +26,9 @@ use path::Path; use ptr; use sync::StaticMutex; use sys::c; - use sys::fs::{OpenOptions, File}; -use sys::handle::{Handle, RawHandle}; +use sys::handle::Handle; +use sys::pipe::{self, AnonPipe}; use sys::stdio; use sys::{self, cvt}; use sys_common::{AsInner, FromInner}; @@ -51,13 +51,28 @@ fn ensure_no_nuls>(str: T) -> io::Result { } } -#[derive(Clone)] pub struct Command { program: OsString, args: Vec, env: Option>, cwd: Option, detach: bool, // not currently exposed in std::process + stdin: Option, + stdout: Option, + stderr: Option, +} + +pub enum Stdio { + Inherit, + Null, + MakePipe, + Handle(Handle), +} + +pub struct StdioPipes { + pub stdin: Option, + pub stdout: Option, + pub stderr: Option, } impl Command { @@ -68,6 +83,9 @@ impl Command { env: None, cwd: None, detach: false, + stdin: None, + stdout: None, + stderr: None, } } @@ -95,56 +113,29 @@ impl Command { pub fn cwd(&mut self, dir: &OsStr) { self.cwd = Some(dir.to_os_string()) } -} - -impl fmt::Debug for Command { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - try!(write!(f, "{:?}", self.program)); - for arg in &self.args { - try!(write!(f, " {:?}", arg)); - } - Ok(()) + pub fn stdin(&mut self, stdin: Stdio) { + self.stdin = Some(stdin); + } + pub fn stdout(&mut self, stdout: Stdio) { + self.stdout = Some(stdout); + } + pub fn stderr(&mut self, stderr: Stdio) { + self.stderr = Some(stderr); } -} - -//////////////////////////////////////////////////////////////////////////////// -// Processes -//////////////////////////////////////////////////////////////////////////////// - -/// A value representing a child process. -/// -/// The lifetime of this value is linked to the lifetime of the actual -/// process - the Process destructor calls self.finish() which waits -/// for the process to terminate. -pub struct Process { - handle: Handle, -} - -pub enum Stdio { - Inherit, - Null, - Raw(c::HANDLE), -} - -pub type RawStdio = Handle; -impl Process { - pub fn spawn(cfg: &Command, - in_handle: Stdio, - out_handle: Stdio, - err_handle: Stdio) -> io::Result - { + pub fn spawn(&mut self, default: Stdio) + -> io::Result<(Process, StdioPipes)> { // To have the spawning semantics of unix/windows stay the same, we need // to read the *child's* PATH if one is provided. See #15149 for more // details. - let program = cfg.env.as_ref().and_then(|env| { + let program = self.env.as_ref().and_then(|env| { for (key, v) in env { if OsStr::new("PATH") != &**key { continue } // Split the value and test each path to see if the // program exists. for path in split_paths(&v) { - let path = path.join(cfg.program.to_str().unwrap()) + let path = path.join(self.program.to_str().unwrap()) .with_extension(env::consts::EXE_EXTENSION); if fs::metadata(&path).is_ok() { return Some(path.into_os_string()) @@ -159,18 +150,18 @@ impl Process { si.cb = mem::size_of::() as c::DWORD; si.dwFlags = c::STARTF_USESTDHANDLES; - let program = program.as_ref().unwrap_or(&cfg.program); - let mut cmd_str = try!(make_command_line(program, &cfg.args)); + let program = program.as_ref().unwrap_or(&self.program); + let mut cmd_str = try!(make_command_line(program, &self.args)); cmd_str.push(0); // add null terminator // stolen from the libuv code. let mut flags = c::CREATE_UNICODE_ENVIRONMENT; - if cfg.detach { + if self.detach { flags |= c::DETACHED_PROCESS | c::CREATE_NEW_PROCESS_GROUP; } - let (envp, _data) = try!(make_envp(cfg.env.as_ref())); - let (dirp, _data) = try!(make_dirp(cfg.cwd.as_ref())); + let (envp, _data) = try!(make_envp(self.env.as_ref())); + let (dirp, _data) = try!(make_dirp(self.cwd.as_ref())); let mut pi = zeroed_process_information(); // Prepare all stdio handles to be inherited by the child. This @@ -185,9 +176,19 @@ impl Process { static CREATE_PROCESS_LOCK: StaticMutex = StaticMutex::new(); let _lock = CREATE_PROCESS_LOCK.lock(); - let stdin = try!(in_handle.to_handle(c::STD_INPUT_HANDLE)); - let stdout = try!(out_handle.to_handle(c::STD_OUTPUT_HANDLE)); - let stderr = try!(err_handle.to_handle(c::STD_ERROR_HANDLE)); + let mut pipes = StdioPipes { + stdin: None, + stdout: None, + stderr: None, + }; + let stdin = self.stdin.as_ref().unwrap_or(&default); + let stdout = self.stdout.as_ref().unwrap_or(&default); + let stderr = self.stderr.as_ref().unwrap_or(&default); + let stdin = try!(stdin.to_handle(c::STD_INPUT_HANDLE, &mut pipes.stdin)); + let stdout = try!(stdout.to_handle(c::STD_OUTPUT_HANDLE, + &mut pipes.stdout)); + let stderr = try!(stderr.to_handle(c::STD_ERROR_HANDLE, + &mut pipes.stderr)); si.hStdInput = stdin.raw(); si.hStdOutput = stdout.raw(); si.hStdError = stderr.raw(); @@ -206,9 +207,92 @@ impl Process { // around to be able to close it later. drop(Handle::new(pi.hThread)); - Ok(Process { handle: Handle::new(pi.hProcess) }) + Ok((Process { handle: Handle::new(pi.hProcess) }, pipes)) + } + +} + +impl fmt::Debug for Command { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + try!(write!(f, "{:?}", self.program)); + for arg in &self.args { + try!(write!(f, " {:?}", arg)); + } + Ok(()) + } +} + +impl Stdio { + fn to_handle(&self, stdio_id: c::DWORD, pipe: &mut Option) + -> io::Result { + match *self { + // If no stdio handle is available, then inherit means that it + // should still be unavailable so propagate the + // INVALID_HANDLE_VALUE. + Stdio::Inherit => { + match stdio::get(stdio_id) { + Ok(io) => io.handle().duplicate(0, true, + c::DUPLICATE_SAME_ACCESS), + Err(..) => Ok(Handle::new(c::INVALID_HANDLE_VALUE)), + } + } + + Stdio::MakePipe => { + let (reader, writer) = try!(pipe::anon_pipe()); + let (ours, theirs) = if stdio_id == c::STD_INPUT_HANDLE { + (writer, reader) + } else { + (reader, writer) + }; + *pipe = Some(ours); + try!(cvt(unsafe { + c::SetHandleInformation(theirs.handle().raw(), + c::HANDLE_FLAG_INHERIT, + c::HANDLE_FLAG_INHERIT) + })); + Ok(theirs.into_handle()) + } + + Stdio::Handle(ref handle) => { + handle.duplicate(0, true, c::DUPLICATE_SAME_ACCESS) + } + + // Open up a reference to NUL with appropriate read/write + // permissions as well as the ability to be inherited to child + // processes (as this is about to be inherited). + Stdio::Null => { + let size = mem::size_of::(); + let mut sa = c::SECURITY_ATTRIBUTES { + nLength: size as c::DWORD, + lpSecurityDescriptor: ptr::null_mut(), + bInheritHandle: 1, + }; + let mut opts = OpenOptions::new(); + opts.read(stdio_id == c::STD_INPUT_HANDLE); + opts.write(stdio_id != c::STD_INPUT_HANDLE); + opts.security_attributes(&mut sa); + File::open(Path::new("NUL"), &opts).map(|file| { + file.into_handle() + }) + } + } } +} + +//////////////////////////////////////////////////////////////////////////////// +// Processes +//////////////////////////////////////////////////////////////////////////////// + +/// A value representing a child process. +/// +/// The lifetime of this value is linked to the lifetime of the actual +/// process - the Process destructor calls self.finish() which waits +/// for the process to terminate. +pub struct Process { + handle: Handle, +} +impl Process { pub fn kill(&mut self) -> io::Result<()> { try!(cvt(unsafe { c::TerminateProcess(self.handle.raw(), 1) @@ -376,45 +460,6 @@ fn make_dirp(d: Option<&OsString>) -> io::Result<(*const u16, Vec)> { } } -impl Stdio { - fn to_handle(&self, stdio_id: c::DWORD) -> io::Result { - match *self { - // If no stdio handle is available, then inherit means that it - // should still be unavailable so propagate the - // INVALID_HANDLE_VALUE. - Stdio::Inherit => { - match stdio::get(stdio_id) { - Ok(io) => io.handle().duplicate(0, true, - c::DUPLICATE_SAME_ACCESS), - Err(..) => Ok(Handle::new(c::INVALID_HANDLE_VALUE)), - } - } - Stdio::Raw(handle) => { - RawHandle::new(handle).duplicate(0, true, c::DUPLICATE_SAME_ACCESS) - } - - // Open up a reference to NUL with appropriate read/write - // permissions as well as the ability to be inherited to child - // processes (as this is about to be inherited). - Stdio::Null => { - let size = mem::size_of::(); - let mut sa = c::SECURITY_ATTRIBUTES { - nLength: size as c::DWORD, - lpSecurityDescriptor: ptr::null_mut(), - bInheritHandle: 1, - }; - let mut opts = OpenOptions::new(); - opts.read(stdio_id == c::STD_INPUT_HANDLE); - opts.write(stdio_id != c::STD_INPUT_HANDLE); - opts.security_attributes(&mut sa); - File::open(Path::new("NUL"), &opts).map(|file| { - file.into_handle() - }) - } - } - } -} - #[cfg(test)] mod tests { use prelude::v1::*; From b37477c03e7683cc67273ddc5506496a7b03971c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 4 Feb 2016 11:16:32 -0800 Subject: [PATCH 7/9] std: Implement CommandExt::exec This commit implements the `exec` function proposed in [RFC 1359][rfc] which is a function on the `CommandExt` trait to execute all parts of a `Command::spawn` without the `fork` on Unix. More details on the function itself can be found in the comments in the commit. [rfc]: https://github.com/rust-lang/rfcs/pull/1359 cc #31398 --- src/libstd/sys/unix/ext/process.rs | 26 +++++++++++ src/libstd/sys/unix/process.rs | 16 ++++++- src/test/run-pass/command-exec.rs | 72 ++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 src/test/run-pass/command-exec.rs diff --git a/src/libstd/sys/unix/ext/process.rs b/src/libstd/sys/unix/ext/process.rs index cf72cfd7e507e..fa19a2620bacf 100644 --- a/src/libstd/sys/unix/ext/process.rs +++ b/src/libstd/sys/unix/ext/process.rs @@ -75,6 +75,28 @@ pub trait CommandExt { #[unstable(feature = "process_exec", issue = "31398")] fn before_exec(&mut self, f: F) -> &mut process::Command where F: FnMut() -> io::Result<()> + Send + Sync + 'static; + + /// Performs all the required setup by this `Command`, followed by calling + /// the `execvp` syscall. + /// + /// On success this function will not return, and otherwise it will return + /// an error indicating why the exec (or another part of the setup of the + /// `Command`) failed. + /// + /// This function, unlike `spawn`, will **not** `fork` the process to create + /// a new child. Like spawn, however, the default behavior for the stdio + /// descriptors will be to inherited from the current process. + /// + /// # Notes + /// + /// The process may be in a "broken state" if this function returns in + /// error. For example the working directory, environment variables, signal + /// handling settings, various user/group information, or aspects of stdio + /// file descriptors may have changed. If a "transactional spawn" is + /// required to gracefully handle errors it is recommended to use the + /// cross-platform `spawn` instead. + #[unstable(feature = "process_exec", issue = "31398")] + fn exec(&mut self) -> io::Error; } #[stable(feature = "rust1", since = "1.0.0")] @@ -100,6 +122,10 @@ impl CommandExt for process::Command { self.as_inner_mut().before_exec(Box::new(f)); self } + + fn exec(&mut self) -> io::Error { + self.as_inner_mut().exec(sys::process::Stdio::Inherit) + } } /// Unix-specific extensions to `std::process::ExitStatus` diff --git a/src/libstd/sys/unix/process.rs b/src/libstd/sys/unix/process.rs index 4716d25c0b28f..60785f376423f 100644 --- a/src/libstd/sys/unix/process.rs +++ b/src/libstd/sys/unix/process.rs @@ -230,7 +230,7 @@ impl Command { match try!(cvt(libc::fork())) { 0 => { drop(input); - let err = self.exec(theirs); + let err = self.do_exec(theirs); let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32; let bytes = [ (errno >> 24) as u8, @@ -290,6 +290,18 @@ impl Command { } } + pub fn exec(&mut self, default: Stdio) -> io::Error { + if self.saw_nul { + return io::Error::new(ErrorKind::InvalidInput, + "nul byte found in provided data") + } + + match self.setup_io(default) { + Ok((_, theirs)) => unsafe { self.do_exec(theirs) }, + Err(e) => e, + } + } + // And at this point we've reached a special time in the life of the // child. The child must now be considered hamstrung and unable to // do anything other than syscalls really. Consider the following @@ -320,7 +332,7 @@ impl Command { // allocation). Instead we just close it manually. This will never // have the drop glue anyway because this code never returns (the // child will either exec() or invoke libc::exit) - unsafe fn exec(&mut self, stdio: ChildPipes) -> io::Error { + unsafe fn do_exec(&mut self, stdio: ChildPipes) -> io::Error { macro_rules! try { ($e:expr) => (match $e { Ok(e) => e, diff --git a/src/test/run-pass/command-exec.rs b/src/test/run-pass/command-exec.rs new file mode 100644 index 0000000000000..039245bfd08ba --- /dev/null +++ b/src/test/run-pass/command-exec.rs @@ -0,0 +1,72 @@ +// Copyright 2016 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// ignore-windows - this is a unix-specific test +// ignore-pretty + +#![feature(process_exec)] + +use std::env; +use std::os::unix::process::CommandExt; +use std::process::Command; + +fn main() { + let mut args = env::args(); + let me = args.next().unwrap(); + + if let Some(arg) = args.next() { + match &arg[..] { + "test1" => println!("passed"), + + "exec-test1" => { + let err = Command::new(&me).arg("test1").exec(); + panic!("failed to spawn: {}", err); + } + + "exec-test2" => { + Command::new("/path/to/nowhere").exec(); + println!("passed"); + } + + "exec-test3" => { + Command::new(&me).arg("bad\0").exec(); + println!("passed"); + } + + "exec-test4" => { + Command::new(&me).current_dir("/path/to/nowhere").exec(); + println!("passed"); + } + + _ => panic!("unknown argument: {}", arg), + } + return + } + + let output = Command::new(&me).arg("exec-test1").output().unwrap(); + assert!(output.status.success()); + assert!(output.stderr.is_empty()); + assert_eq!(output.stdout, b"passed\n"); + + let output = Command::new(&me).arg("exec-test2").output().unwrap(); + assert!(output.status.success()); + assert!(output.stderr.is_empty()); + assert_eq!(output.stdout, b"passed\n"); + + let output = Command::new(&me).arg("exec-test3").output().unwrap(); + assert!(output.status.success()); + assert!(output.stderr.is_empty()); + assert_eq!(output.stdout, b"passed\n"); + + let output = Command::new(&me).arg("exec-test4").output().unwrap(); + assert!(output.status.success()); + assert!(output.stderr.is_empty()); + assert_eq!(output.stdout, b"passed\n"); +} From efb23db79a5cc16770a7c3d5cef7059d868dea8f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 9 Feb 2016 15:13:33 -0800 Subject: [PATCH 8/9] std: Use macros from libc instead of locally Helps cut down on #[cfg]! --- src/libstd/sys/unix/process.rs | 27 +++------------------------ 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/src/libstd/sys/unix/process.rs b/src/libstd/sys/unix/process.rs index 60785f376423f..9e12b2f116cc3 100644 --- a/src/libstd/sys/unix/process.rs +++ b/src/libstd/sys/unix/process.rs @@ -519,30 +519,9 @@ impl fmt::Debug for Command { #[derive(PartialEq, Eq, Clone, Copy, Debug)] pub struct ExitStatus(c_int); -#[cfg(any(target_os = "linux", target_os = "android", - target_os = "nacl"))] -mod status_imp { - pub fn WIFEXITED(status: i32) -> bool { (status & 0xff) == 0 } - pub fn WEXITSTATUS(status: i32) -> i32 { (status >> 8) & 0xff } - pub fn WTERMSIG(status: i32) -> i32 { status & 0x7f } -} - -#[cfg(any(target_os = "macos", - target_os = "ios", - target_os = "freebsd", - target_os = "dragonfly", - target_os = "bitrig", - target_os = "netbsd", - target_os = "openbsd"))] -mod status_imp { - pub fn WIFEXITED(status: i32) -> bool { (status & 0x7f) == 0 } - pub fn WEXITSTATUS(status: i32) -> i32 { status >> 8 } - pub fn WTERMSIG(status: i32) -> i32 { status & 0o177 } -} - impl ExitStatus { fn exited(&self) -> bool { - status_imp::WIFEXITED(self.0) + unsafe { libc::WIFEXITED(self.0) } } pub fn success(&self) -> bool { @@ -551,7 +530,7 @@ impl ExitStatus { pub fn code(&self) -> Option { if self.exited() { - Some(status_imp::WEXITSTATUS(self.0)) + Some(unsafe { libc::WEXITSTATUS(self.0) }) } else { None } @@ -559,7 +538,7 @@ impl ExitStatus { pub fn signal(&self) -> Option { if !self.exited() { - Some(status_imp::WTERMSIG(self.0)) + Some(unsafe { libc::WTERMSIG(self.0) }) } else { None } From d9c6a51c3b4b7f215a4cfcb7d3e1921fe2fa9657 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 9 Feb 2016 15:14:12 -0800 Subject: [PATCH 9/9] std: Move constant back to where it needs to be Lost track of this during the std::process refactorings --- src/libstd/sys/unix/process.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstd/sys/unix/process.rs b/src/libstd/sys/unix/process.rs index 9e12b2f116cc3..28475f50ce63e 100644 --- a/src/libstd/sys/unix/process.rs +++ b/src/libstd/sys/unix/process.rs @@ -218,6 +218,8 @@ impl Command { pub fn spawn(&mut self, default: Stdio) -> io::Result<(Process, StdioPipes)> { + const CLOEXEC_MSG_FOOTER: &'static [u8] = b"NOEX"; + if self.saw_nul { return Err(io::Error::new(ErrorKind::InvalidInput, "nul byte found in provided data")); @@ -562,8 +564,6 @@ pub struct Process { status: Option, } -const CLOEXEC_MSG_FOOTER: &'static [u8] = b"NOEX"; - impl Process { pub fn id(&self) -> u32 { self.pid as u32