From bd8b4b9c1588f0c681162ead34cc91b3f9a52bfc Mon Sep 17 00:00:00 2001 From: Rain Date: Fri, 19 Aug 2022 18:24:21 -0700 Subject: [PATCH] Use posix_spawn for absolute paths on macOS Currently, on macOS, Rust never uses the fast posix_spawn path if a directory change is requested due to a bug in Apple's libc. However, the bug is only triggered if the program is a relative path. This PR makes it so that the fast path continues to work if the program is an absolute path or a lone filename. This was an alternative proposed in https://github.com/rust-lang/rust/pull/80537#issue-776674009, and it makes a measurable performance difference in some of my code that spawns thousands of processes. --- .../src/sys/unix/process/process_common.rs | 33 +++++++++++++++++++ .../sys/unix/process/process_common/tests.rs | 24 ++++++++++++++ .../std/src/sys/unix/process/process_unix.rs | 4 ++- 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/process/process_common.rs b/library/std/src/sys/unix/process/process_common.rs index 6985f1d783087..2834ee0ace835 100644 --- a/library/std/src/sys/unix/process/process_common.rs +++ b/library/std/src/sys/unix/process/process_common.rs @@ -92,6 +92,7 @@ pub struct Command { argv: Argv, env: CommandEnv, + program_kind: ProgramKind, cwd: Option, uid: Option, gid: Option, @@ -148,15 +149,40 @@ pub enum Stdio { Fd(FileDesc), } +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum ProgramKind { + /// A program that would be looked up on the PATH (e.g. `ls`) + PathLookup, + /// A relative path (e.g. `my-dir/foo`, `../foo`, `./foo`) + Relative, + /// An absolute path. + Absolute, +} + +impl ProgramKind { + fn new(program: &OsStr) -> Self { + if program.bytes().starts_with(b"/") { + Self::Absolute + } else if program.bytes().contains(&b'/') { + // If the program has more than one component in it, it is a relative path. + Self::Relative + } else { + Self::PathLookup + } + } +} + impl Command { #[cfg(not(target_os = "linux"))] pub fn new(program: &OsStr) -> Command { let mut saw_nul = false; + let program_kind = ProgramKind::new(program.as_ref()); let program = os2c(program, &mut saw_nul); Command { argv: Argv(vec![program.as_ptr(), ptr::null()]), args: vec![program.clone()], program, + program_kind, env: Default::default(), cwd: None, uid: None, @@ -174,11 +200,13 @@ impl Command { #[cfg(target_os = "linux")] pub fn new(program: &OsStr) -> Command { let mut saw_nul = false; + let program_kind = ProgramKind::new(program.as_ref()); let program = os2c(program, &mut saw_nul); Command { argv: Argv(vec![program.as_ptr(), ptr::null()]), args: vec![program.clone()], program, + program_kind, env: Default::default(), cwd: None, uid: None, @@ -254,6 +282,11 @@ impl Command { OsStr::from_bytes(self.program.as_bytes()) } + #[allow(dead_code)] + pub fn get_program_kind(&self) -> ProgramKind { + self.program_kind + } + pub fn get_args(&self) -> CommandArgs<'_> { let mut iter = self.args.iter(); iter.next(); diff --git a/library/std/src/sys/unix/process/process_common/tests.rs b/library/std/src/sys/unix/process/process_common/tests.rs index 1956b3692a7ea..d176b3401c03c 100644 --- a/library/std/src/sys/unix/process/process_common/tests.rs +++ b/library/std/src/sys/unix/process/process_common/tests.rs @@ -122,3 +122,27 @@ fn test_process_group_no_posix_spawn() { t!(cat.wait()); } } + +#[test] +fn test_program_kind() { + let vectors = &[ + ("foo", ProgramKind::PathLookup), + ("foo.out", ProgramKind::PathLookup), + ("./foo", ProgramKind::Relative), + ("../foo", ProgramKind::Relative), + ("dir/foo", ProgramKind::Relative), + // Note that paths on Unix can't contain / in them, so this is actually the directory "fo\\" + // followed by the file "o". + ("fo\\/o", ProgramKind::Relative), + ("/foo", ProgramKind::Absolute), + ("/dir/../foo", ProgramKind::Absolute), + ]; + + for (program, expected_kind) in vectors { + assert_eq!( + ProgramKind::new(program.as_ref()), + *expected_kind, + "actual != expected program kind for input {program}", + ); + } +} diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 75bb92437fd92..26ae62817713e 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -453,7 +453,9 @@ impl Command { // successfully launch the program, but erroneously return // ENOENT when used with posix_spawn_file_actions_addchdir_np // which was introduced in macOS 10.15. - return Ok(None); + if self.get_program_kind() == ProgramKind::Relative { + return Ok(None); + } } match posix_spawn_file_actions_addchdir_np.get() { Some(f) => Some((f, cwd)),