diff --git a/src/libstd/sys/redox/ext/process.rs b/src/libstd/sys/redox/ext/process.rs index 1dcc1169510f3..020075531dd0e 100644 --- a/src/libstd/sys/redox/ext/process.rs +++ b/src/libstd/sys/redox/ext/process.rs @@ -36,7 +36,7 @@ pub trait CommandExt { /// will be called and the spawn operation will immediately return with a /// failure. /// - /// # Notes + /// # Notes and Safety /// /// This closure will be run in the context of the child process after a /// `fork`. This primarily means that any modifications made to memory on @@ -45,12 +45,32 @@ pub trait CommandExt { /// like `malloc` or acquiring a mutex are not guaranteed to work (due to /// other threads perhaps still running when the `fork` was run). /// + /// This also means that all resources such as file descriptors and + /// memory-mapped regions got duplicated. It is your responsibility to make + /// sure that the closure does not violate library invariants by making + /// invalid use of these duplicates. + /// /// 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. + #[stable(feature = "process_pre_exec", since = "1.34.0")] + unsafe fn pre_exec(&mut self, f: F) -> &mut process::Command + where F: FnMut() -> io::Result<()> + Send + Sync + 'static; + + /// Schedules a closure to be run just before the `exec` function is + /// invoked. + /// + /// This method is stable and usable, but it should be unsafe. To fix + /// that, it got deprecated in favor of the unsafe [`pre_exec`]. + /// + /// [`pre_exec`]: #tymethod.pre_exec #[stable(feature = "process_exec", since = "1.15.0")] + #[rustc_deprecated(since = "1.37.0", reason = "should be unsafe, use `pre_exec` instead")] fn before_exec(&mut self, f: F) -> &mut process::Command - where F: FnMut() -> io::Result<()> + Send + Sync + 'static; + where F: FnMut() -> io::Result<()> + Send + Sync + 'static + { + unsafe { self.pre_exec(f) } + } /// Performs all the required setup by this `Command`, followed by calling /// the `execvp` syscall. @@ -87,10 +107,10 @@ impl CommandExt for process::Command { self } - fn before_exec(&mut self, f: F) -> &mut process::Command + unsafe fn pre_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.as_inner_mut().pre_exec(Box::new(f)); self } diff --git a/src/libstd/sys/redox/process.rs b/src/libstd/sys/redox/process.rs index 9e23c537f22d4..81af8eb553d00 100644 --- a/src/libstd/sys/redox/process.rs +++ b/src/libstd/sys/redox/process.rs @@ -116,8 +116,10 @@ impl Command { self.gid = Some(id); } - pub fn before_exec(&mut self, - f: Box io::Result<()> + Send + Sync>) { + pub unsafe fn pre_exec( + &mut self, + f: Box io::Result<()> + Send + Sync>, + ) { self.closures.push(f); } diff --git a/src/libstd/sys/unix/ext/process.rs b/src/libstd/sys/unix/ext/process.rs index 2c5943fdac348..b487bb889bafa 100644 --- a/src/libstd/sys/unix/ext/process.rs +++ b/src/libstd/sys/unix/ext/process.rs @@ -36,7 +36,7 @@ pub trait CommandExt { /// will be called and the spawn operation will immediately return with a /// failure. /// - /// # Notes + /// # Notes and Safety /// /// This closure will be run in the context of the child process after a /// `fork`. This primarily means that any modifications made to memory on @@ -45,12 +45,32 @@ pub trait CommandExt { /// like `malloc` or acquiring a mutex are not guaranteed to work (due to /// other threads perhaps still running when the `fork` was run). /// + /// This also means that all resources such as file descriptors and + /// memory-mapped regions got duplicated. It is your responsibility to make + /// sure that the closure does not violate library invariants by making + /// invalid use of these duplicates. + /// /// 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. + #[stable(feature = "process_pre_exec", since = "1.34.0")] + unsafe fn pre_exec(&mut self, f: F) -> &mut process::Command + where F: FnMut() -> io::Result<()> + Send + Sync + 'static; + + /// Schedules a closure to be run just before the `exec` function is + /// invoked. + /// + /// This method is stable and usable, but it should be unsafe. To fix + /// that, it got deprecated in favor of the unsafe [`pre_exec`]. + /// + /// [`pre_exec`]: #tymethod.pre_exec #[stable(feature = "process_exec", since = "1.15.0")] + #[rustc_deprecated(since = "1.37.0", reason = "should be unsafe, use `pre_exec` instead")] fn before_exec(&mut self, f: F) -> &mut process::Command - where F: FnMut() -> io::Result<()> + Send + Sync + 'static; + where F: FnMut() -> io::Result<()> + Send + Sync + 'static + { + unsafe { self.pre_exec(f) } + } /// Performs all the required setup by this `Command`, followed by calling /// the `execvp` syscall. @@ -97,10 +117,10 @@ impl CommandExt for process::Command { self } - fn before_exec(&mut self, f: F) -> &mut process::Command + unsafe fn pre_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.as_inner_mut().pre_exec(Box::new(f)); self } diff --git a/src/libstd/sys/unix/process/process_common.rs b/src/libstd/sys/unix/process/process_common.rs index 2c55813c5cd39..7fa256e59b2db 100644 --- a/src/libstd/sys/unix/process/process_common.rs +++ b/src/libstd/sys/unix/process/process_common.rs @@ -149,8 +149,10 @@ impl Command { &mut self.closures } - pub fn before_exec(&mut self, - f: Box io::Result<()> + Send + Sync>) { + pub unsafe fn pre_exec( + &mut self, + f: Box io::Result<()> + Send + Sync>, + ) { self.closures.push(f); } diff --git a/src/test/run-pass/command-before-exec.rs b/src/test/run-pass/command-before-exec.rs deleted file mode 100644 index 91d2636b2ae63..0000000000000 --- a/src/test/run-pass/command-before-exec.rs +++ /dev/null @@ -1,83 +0,0 @@ -#![allow(stable_features)] -// ignore-windows - this is a unix-specific test -// ignore-cloudabi no processes -// ignore-emscripten no processes - -#![feature(process_exec, rustc_private)] - -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().nth(1) { - 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().unwrap_err(); - 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); -} diff --git a/src/test/run-pass/command-pre-exec.rs b/src/test/run-pass/command-pre-exec.rs new file mode 100644 index 0000000000000..21783fedd39c9 --- /dev/null +++ b/src/test/run-pass/command-pre-exec.rs @@ -0,0 +1,115 @@ +#![allow(stable_features)] +// ignore-windows - this is a unix-specific test +// ignore-cloudabi no processes +// ignore-emscripten no processes +#![feature(process_exec, rustc_private)] + +extern crate libc; + +use std::env; +use std::io::Error; +use std::os::unix::process::CommandExt; +use std::process::Command; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::Arc; + +fn main() { + if let Some(arg) = env::args().nth(1) { + 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 = unsafe { + Command::new(&me) + .arg("test1") + .pre_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 = unsafe { + Command::new(&me) + .arg("test2") + .pre_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 = unsafe { + Command::new(&me) + .arg("test3") + .pre_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 = unsafe { + Command::new(&me) + .arg("bad") + .pre_exec(|| Err(Error::from_raw_os_error(102))) + .output() + .unwrap_err() + }; + assert_eq!(output.raw_os_error(), Some(102)); + + let pid = unsafe { libc::getpid() }; + assert!(pid >= 0); + let output = unsafe { + Command::new(&me) + .arg("empty") + .pre_exec(move || { + let child = 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 = unsafe { + Command::new(&me) + .arg("empty") + .pre_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); +}