Skip to content

Commit 7314b73

Browse files
committed
more fine-grained feature-detection for pidfd spawning
we now distinguish between pidfd_spawn support, pidfd-via-fork/exec and not-supported
1 parent a2f42c0 commit 7314b73

File tree

1 file changed

+34
-21
lines changed

1 file changed

+34
-21
lines changed

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

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,7 @@ 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+
use core::assert_matches::debug_assert_matches;
452453
#[cfg(target_os = "linux")]
453454
use core::sync::atomic::{AtomicU8, Ordering};
454455

@@ -476,35 +477,47 @@ impl Command {
476477

477478
weak! { fn pidfd_getpid(libc::c_int) -> libc::c_int }
478479

479-
static PIDFD_SPAWN_SUPPORTED: AtomicU8 = AtomicU8::new(0);
480+
static PIDFD_SUPPORTED: AtomicU8 = AtomicU8::new(0);
480481
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;
482+
const SPAWN: u8 = 1;
483+
// Obtaining a pidfd via the fork+exec path might work
484+
const FORK_EXEC: u8 = 2;
485+
// Neither pidfd_spawn nor fork/exec will get us a pidfd.
486+
// Instead we'll just posix_spawn if the other preconditions are met.
487+
const NO: u8 = 3;
485488

486489
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() {
490+
let mut support = PIDFD_SUPPORTED.load(Ordering::Relaxed);
491+
if support == FORK_EXEC {
489492
return Ok(None);
490493
}
491-
if flag == UNKNOWN {
492-
let mut support = NO;
494+
if support == UNKNOWN {
495+
support = NO;
493496
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-
};
497+
let pidfd = cvt(unsafe { libc::syscall(libc::SYS_pidfd_open, our_pid, 0) } as c_int);
498+
match pidfd {
499+
Ok(pidfd) => {
500+
support = FORK_EXEC;
501+
if let Some(Ok(pid)) = pidfd_getpid.get().map(|f| cvt(unsafe { f(pidfd) } as i32)) {
502+
if pidfd_spawnp.get().is_some() && pid as u32 == our_pid {
503+
support = SPAWN
504+
}
505+
}
506+
unsafe { libc::close(pidfd) };
507+
}
508+
Err(e) if e.raw_os_error() == Some(libc::EMFILE) => {
509+
// We're temporarily(?) out of file descriptors. In this case obtaining a pidfd would also fail
510+
// Don't update the support flag so we can probe again later.
511+
return Err(e)
512+
}
513+
_ => {}
502514
}
503-
PIDFD_SPAWN_SUPPORTED.store(support, Ordering::Relaxed);
504-
if support != YES {
515+
PIDFD_SUPPORTED.store(support, Ordering::Relaxed);
516+
if support == FORK_EXEC {
505517
return Ok(None);
506518
}
507519
}
520+
debug_assert_matches!(support, SPAWN | NO);
508521
}
509522
} else {
510523
if self.get_create_pidfd() {
@@ -691,7 +704,7 @@ impl Command {
691704
let spawn_fn = retrying_libc_posix_spawnp;
692705

693706
#[cfg(target_os = "linux")]
694-
if self.get_create_pidfd() {
707+
if self.get_create_pidfd() && PIDFD_SUPPORTED.load(Ordering::Relaxed) == SPAWN {
695708
let mut pidfd: libc::c_int = -1;
696709
let spawn_res = pidfd_spawnp.get().unwrap()(
697710
&mut pidfd,
@@ -706,7 +719,7 @@ impl Command {
706719
if let Err(ref e) = spawn_res
707720
&& e.raw_os_error() == Some(libc::ENOSYS)
708721
{
709-
PIDFD_SPAWN_SUPPORTED.store(NO, Ordering::Relaxed);
722+
PIDFD_SUPPORTED.store(FORK_EXEC, Ordering::Relaxed);
710723
return Ok(None);
711724
}
712725
spawn_res?;

0 commit comments

Comments
 (0)