Skip to content

Commit 9212236

Browse files
committed
use pidfd_spawn for faster process creation when pidfds are requested
1 parent 4815f29 commit 9212236

File tree

2 files changed

+106
-6
lines changed

2 files changed

+106
-6
lines changed

std/src/sys/pal/unix/linux/pidfd/tests.rs

+11-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::assert_matches::assert_matches;
22
use crate::os::fd::{AsRawFd, RawFd};
3-
use crate::os::linux::process::{ChildExt, CommandExt};
4-
use crate::os::unix::process::ExitStatusExt;
3+
use crate::os::linux::process::{ChildExt, CommandExt as _};
4+
use crate::os::unix::process::{CommandExt as _, ExitStatusExt};
55
use crate::process::Command;
66

77
#[test]
@@ -42,6 +42,15 @@ fn test_command_pidfd() {
4242
.unwrap()
4343
.pidfd()
4444
.expect_err("pidfd should not have been created");
45+
46+
// exercise the fork/exec path since the earlier attempts may have used pidfd_spawnp()
47+
let mut child =
48+
unsafe { Command::new("false").pre_exec(|| Ok(())) }.create_pidfd(true).spawn().unwrap();
49+
50+
if pidfd_open_available {
51+
assert!(child.pidfd().is_ok())
52+
}
53+
child.wait().expect("error waiting on child");
4554
}
4655

4756
#[test]

std/src/sys/pal/unix/process/process_unix.rs

+95-4
Original file line numberDiff line numberDiff line change
@@ -449,17 +449,70 @@ impl Command {
449449
use crate::mem::MaybeUninit;
450450
use crate::sys::weak::weak;
451451
use crate::sys::{self, cvt_nz, on_broken_pipe_flag_used};
452+
#[cfg(target_os = "linux")]
453+
use core::sync::atomic::{AtomicU8, Ordering};
452454

453455
if self.get_gid().is_some()
454456
|| self.get_uid().is_some()
455457
|| (self.env_saw_path() && !self.program_is_path())
456458
|| !self.get_closures().is_empty()
457459
|| self.get_groups().is_some()
458-
|| self.get_create_pidfd()
459460
{
460461
return Ok(None);
461462
}
462463

464+
cfg_if::cfg_if! {
465+
if #[cfg(target_os = "linux")] {
466+
weak! {
467+
fn pidfd_spawnp(
468+
*mut libc::c_int,
469+
*const libc::c_char,
470+
*const libc::posix_spawn_file_actions_t,
471+
*const libc::posix_spawnattr_t,
472+
*const *mut libc::c_char,
473+
*const *mut libc::c_char
474+
) -> libc::c_int
475+
}
476+
477+
weak! { fn pidfd_getpid(libc::c_int) -> libc::c_int }
478+
479+
static PIDFD_SPAWN_SUPPORTED: AtomicU8 = AtomicU8::new(0);
480+
const UNKNOWN: u8 = 0;
481+
const YES: u8 = 1;
482+
// NO currently forces a fallback to fork/exec. We could be more nuanced here and keep using spawn
483+
// if we know pidfd's aren't supported at all and the fallback would be futile.
484+
const NO: u8 = 2;
485+
486+
if self.get_create_pidfd() {
487+
let flag = PIDFD_SPAWN_SUPPORTED.load(Ordering::Relaxed);
488+
if flag == NO || pidfd_spawnp.get().is_none() || pidfd_getpid.get().is_none() {
489+
return Ok(None);
490+
}
491+
if flag == UNKNOWN {
492+
let mut support = NO;
493+
let our_pid = crate::process::id();
494+
let pidfd =
495+
unsafe { libc::syscall(libc::SYS_pidfd_open, our_pid, 0) } as libc::c_int;
496+
if pidfd >= 0 {
497+
let pid = unsafe { pidfd_getpid.get().unwrap()(pidfd) } as u32;
498+
unsafe { libc::close(pidfd) };
499+
if pid == our_pid {
500+
support = YES
501+
};
502+
}
503+
PIDFD_SPAWN_SUPPORTED.store(support, Ordering::Relaxed);
504+
if support != YES {
505+
return Ok(None);
506+
}
507+
}
508+
}
509+
} else {
510+
if self.get_create_pidfd() {
511+
unreachable!("only implemented on linux")
512+
}
513+
}
514+
}
515+
463516
// Only glibc 2.24+ posix_spawn() supports returning ENOENT directly.
464517
#[cfg(all(target_os = "linux", target_env = "gnu"))]
465518
{
@@ -543,9 +596,6 @@ impl Command {
543596

544597
let pgroup = self.get_pgroup();
545598

546-
// Safety: -1 indicates we don't have a pidfd.
547-
let mut p = unsafe { Process::new(0, -1) };
548-
549599
struct PosixSpawnFileActions<'a>(&'a mut MaybeUninit<libc::posix_spawn_file_actions_t>);
550600

551601
impl Drop for PosixSpawnFileActions<'_> {
@@ -640,6 +690,47 @@ impl Command {
640690
#[cfg(target_os = "nto")]
641691
let spawn_fn = retrying_libc_posix_spawnp;
642692

693+
#[cfg(target_os = "linux")]
694+
if self.get_create_pidfd() {
695+
let mut pidfd: libc::c_int = -1;
696+
let spawn_res = pidfd_spawnp.get().unwrap()(
697+
&mut pidfd,
698+
self.get_program_cstr().as_ptr(),
699+
file_actions.0.as_ptr(),
700+
attrs.0.as_ptr(),
701+
self.get_argv().as_ptr() as *const _,
702+
envp as *const _,
703+
);
704+
705+
let spawn_res = cvt_nz(spawn_res);
706+
if let Err(ref e) = spawn_res
707+
&& e.raw_os_error() == Some(libc::ENOSYS)
708+
{
709+
PIDFD_SPAWN_SUPPORTED.store(NO, Ordering::Relaxed);
710+
return Ok(None);
711+
}
712+
spawn_res?;
713+
714+
let pid = match cvt(pidfd_getpid.get().unwrap()(pidfd)) {
715+
Ok(pid) => pid,
716+
Err(e) => {
717+
// The child has been spawned and we are holding its pidfd.
718+
// But we cannot obtain its pid even though pidfd_getpid support was verified earlier.
719+
// This might happen if libc can't open procfs because the file descriptor limit has been reached.
720+
libc::close(pidfd);
721+
return Err(Error::new(
722+
e.kind(),
723+
"pidfd_spawnp succeeded but the child's PID could not be obtained",
724+
));
725+
}
726+
};
727+
728+
return Ok(Some(Process::new(pid, pidfd)));
729+
}
730+
731+
// Safety: -1 indicates we don't have a pidfd.
732+
let mut p = Process::new(0, -1);
733+
643734
let spawn_res = spawn_fn(
644735
&mut p.pid,
645736
self.get_program_cstr().as_ptr(),

0 commit comments

Comments
 (0)