Skip to content

Commit cf9a6d6

Browse files
committed
use PIDFD_GET_INFO ioctl when available
This way using pidfd_spawnp won't have to rely on procfs, avoiding an unpleasant edge-case where the child is pawned but we can't get the pid. And pidfd.{try_}wait will be able to return the exit status even after a process has been reaped. At least on newer kernels.
1 parent 82dd3cb commit cf9a6d6

File tree

4 files changed

+126
-42
lines changed

4 files changed

+126
-42
lines changed

library/std/src/os/linux/process.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,10 @@ impl PidFd {
6767
/// Waits for the child to exit completely, returning the status that it exited with.
6868
///
6969
/// Unlike [`Child::wait`] it does not ensure that the stdin handle is closed.
70-
/// Additionally it will not return an `ExitStatus` if the child
71-
/// has already been reaped. Instead an error will be returned.
70+
///
71+
/// Additionally on kernels prior to 6.15 only the first attempt to
72+
/// reap a child will return an ExitStatus, further attempts
73+
/// will return an Error.
7274
///
7375
/// [`Child::wait`]: process::Child::wait
7476
pub fn wait(&self) -> Result<ExitStatus> {
@@ -77,8 +79,8 @@ impl PidFd {
7779

7880
/// Attempts to collect the exit status of the child if it has already exited.
7981
///
80-
/// Unlike [`Child::try_wait`] this method will return an Error
81-
/// if the child has already been reaped.
82+
/// On kernels prior to 6.15, and unlike [`Child::try_wait`], only the first attempt
83+
/// to reap a child will return an ExitStatus, further attempts will return an Error.
8284
///
8385
/// [`Child::try_wait`]: process::Child::try_wait
8486
pub fn try_wait(&self) -> Result<Option<ExitStatus>> {

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

Lines changed: 74 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use crate::io;
2-
use crate::os::fd::{AsRawFd, FromRawFd, RawFd};
2+
use crate::os::fd::{AsRawFd, FromRawFd, IntoRawFd, RawFd};
33
use crate::sys::fd::FileDesc;
44
use crate::sys::process::ExitStatus;
5+
use crate::sys::weak::weak;
56
use crate::sys::{AsInner, FromInner, IntoInner, cvt};
67

78
#[cfg(test)]
@@ -15,6 +16,46 @@ impl PidFd {
1516
self.send_signal(libc::SIGKILL)
1617
}
1718

19+
pub(crate) fn current_process() -> io::Result<PidFd> {
20+
let pid = crate::process::id();
21+
let pidfd = cvt(unsafe { libc::syscall(libc::SYS_pidfd_open, pid, 0) })?;
22+
Ok(unsafe { PidFd::from_raw_fd(pidfd as RawFd) })
23+
}
24+
25+
pub(crate) fn pid(&self) -> io::Result<u32> {
26+
// since kernel 6.13
27+
// https://lore.kernel.org/all/20241010155401.2268522-1-luca.boccassi@gmail.com/
28+
let mut pidfd_info: libc::pidfd_info = unsafe { crate::mem::zeroed() };
29+
pidfd_info.mask = libc::PIDFD_INFO_PID as u64;
30+
match cvt(unsafe { libc::ioctl(self.0.as_raw_fd(), libc::PIDFD_GET_INFO, &mut pidfd_info) })
31+
{
32+
Ok(_) => {}
33+
Err(e) if e.raw_os_error() == Some(libc::EINVAL) => {
34+
// kernel doesn't support that ioctl, try the glibc helper that looks at procfs
35+
weak!(
36+
fn pidfd_getpid(pidfd: libc::c_int) -> libc::c_int;
37+
);
38+
if let Some(pidfd_getpid) = pidfd_getpid.get() {
39+
let pid: libc::c_int = cvt(unsafe { pidfd_getpid(self.0.as_raw_fd()) })?;
40+
return Ok(pid as u32);
41+
}
42+
return Err(e);
43+
}
44+
Err(e) => return Err(e),
45+
}
46+
47+
Ok(pidfd_info.pid)
48+
}
49+
50+
fn exit_for_reaped_child(&self) -> io::Result<ExitStatus> {
51+
// since kernel 6.15
52+
// https://lore.kernel.org/linux-fsdevel/20250305-work-pidfs-kill_on_last_close-v3-0-c8c3d8361705@kernel.org/T/
53+
let mut pidfd_info: libc::pidfd_info = unsafe { crate::mem::zeroed() };
54+
pidfd_info.mask = libc::PIDFD_INFO_EXIT as u64;
55+
cvt(unsafe { libc::ioctl(self.0.as_raw_fd(), libc::PIDFD_GET_INFO, &mut pidfd_info) })?;
56+
Ok(ExitStatus::new(pidfd_info.exit_code))
57+
}
58+
1859
pub(crate) fn send_signal(&self, signal: i32) -> io::Result<()> {
1960
cvt(unsafe {
2061
libc::syscall(
@@ -30,23 +71,44 @@ impl PidFd {
3071

3172
pub fn wait(&self) -> io::Result<ExitStatus> {
3273
let mut siginfo: libc::siginfo_t = unsafe { crate::mem::zeroed() };
33-
cvt(unsafe {
74+
let r = cvt(unsafe {
3475
libc::waitid(libc::P_PIDFD, self.0.as_raw_fd() as u32, &mut siginfo, libc::WEXITED)
35-
})?;
76+
});
77+
match r {
78+
Err(waitid_err) if waitid_err.raw_os_error() == Some(libc::ECHILD) => {
79+
// already reaped
80+
match self.exit_for_reaped_child() {
81+
Ok(exit_status) => return Ok(exit_status),
82+
Err(_) => return Err(waitid_err),
83+
}
84+
}
85+
Err(e) => return Err(e),
86+
Ok(_) => {}
87+
}
3688
Ok(ExitStatus::from_waitid_siginfo(siginfo))
3789
}
3890

3991
pub fn try_wait(&self) -> io::Result<Option<ExitStatus>> {
4092
let mut siginfo: libc::siginfo_t = unsafe { crate::mem::zeroed() };
41-
42-
cvt(unsafe {
93+
let r = cvt(unsafe {
4394
libc::waitid(
4495
libc::P_PIDFD,
4596
self.0.as_raw_fd() as u32,
4697
&mut siginfo,
4798
libc::WEXITED | libc::WNOHANG,
4899
)
49-
})?;
100+
});
101+
match r {
102+
Err(waitid_err) if waitid_err.raw_os_error() == Some(libc::ECHILD) => {
103+
// already reaped
104+
match self.exit_for_reaped_child() {
105+
Ok(exit_status) => return Ok(Some(exit_status)),
106+
Err(_) => return Err(waitid_err),
107+
}
108+
}
109+
Err(e) => return Err(e),
110+
Ok(_) => {}
111+
}
50112
if unsafe { siginfo.si_pid() } == 0 {
51113
Ok(None)
52114
} else {
@@ -78,3 +140,9 @@ impl FromRawFd for PidFd {
78140
Self(FileDesc::from_raw_fd(fd))
79141
}
80142
}
143+
144+
impl IntoRawFd for PidFd {
145+
fn into_raw_fd(self) -> RawFd {
146+
self.0.into_raw_fd()
147+
}
148+
}

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

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1+
use super::PidFd as InternalPidFd;
12
use crate::assert_matches::assert_matches;
2-
use crate::os::fd::{AsRawFd, RawFd};
3+
use crate::io::ErrorKind;
4+
use crate::os::fd::AsRawFd;
35
use crate::os::linux::process::{ChildExt, CommandExt as _};
46
use crate::os::unix::process::{CommandExt as _, ExitStatusExt};
57
use crate::process::Command;
8+
use crate::sys::AsInner;
69

710
#[test]
811
fn test_command_pidfd() {
@@ -48,11 +51,22 @@ fn test_command_pidfd() {
4851
let mut cmd = Command::new("false");
4952
let mut child = unsafe { cmd.pre_exec(|| Ok(())) }.create_pidfd(true).spawn().unwrap();
5053

51-
assert!(child.id() > 0 && child.id() < -1i32 as u32);
54+
let id = child.id();
55+
56+
assert!(id > 0 && id < -1i32 as u32, "spawning with pidfd still returns a sane pid");
5257

5358
if pidfd_open_available {
5459
assert!(child.pidfd().is_ok())
5560
}
61+
62+
if let Ok(pidfd) = child.pidfd() {
63+
match pidfd.as_inner().pid() {
64+
Ok(pid) => assert_eq!(pid, id),
65+
Err(e) if e.kind() == ErrorKind::InvalidInput => { /* older kernel */ }
66+
Err(e) => panic!("unexpected error getting pid from pidfd: {}", e),
67+
}
68+
}
69+
5670
child.wait().expect("error waiting on child");
5771
}
5872

@@ -77,23 +91,21 @@ fn test_pidfd() {
7791
assert_eq!(status.signal(), Some(libc::SIGKILL));
7892

7993
// Trying to wait again for a reaped child is safe since there's no pid-recycling race.
80-
// But doing so will return an error.
94+
// But doing so may return an error.
8195
let res = fd.wait();
82-
assert_matches!(res, Err(e) if e.raw_os_error() == Some(libc::ECHILD));
96+
match res {
97+
// older kernels
98+
Err(e) if e.raw_os_error() == Some(libc::ECHILD) => {}
99+
// 6.15+
100+
Ok(exit) if exit.signal() == Some(libc::SIGKILL) => {}
101+
other => panic!("expected ECHILD error, got {:?}", other),
102+
}
83103

84104
// Ditto for additional attempts to kill an already-dead child.
85105
let res = fd.kill();
86106
assert_matches!(res, Err(e) if e.raw_os_error() == Some(libc::ESRCH));
87107
}
88108

89109
fn probe_pidfd_support() -> bool {
90-
// pidfds require the pidfd_open syscall
91-
let our_pid = crate::process::id();
92-
let pidfd = unsafe { libc::syscall(libc::SYS_pidfd_open, our_pid, 0) };
93-
if pidfd >= 0 {
94-
unsafe { libc::close(pidfd as RawFd) };
95-
true
96-
} else {
97-
false
98-
}
110+
InternalPidFd::current_process().is_ok()
99111
}

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

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -482,10 +482,6 @@ impl Command {
482482
) -> libc::c_int;
483483
);
484484

485-
weak!(
486-
fn pidfd_getpid(pidfd: libc::c_int) -> libc::c_int;
487-
);
488-
489485
static PIDFD_SUPPORTED: Atomic<u8> = AtomicU8::new(0);
490486
const UNKNOWN: u8 = 0;
491487
const SPAWN: u8 = 1;
@@ -502,24 +498,26 @@ impl Command {
502498
}
503499
if support == UNKNOWN {
504500
support = NO;
505-
let our_pid = crate::process::id();
506-
let pidfd = cvt(unsafe { libc::syscall(libc::SYS_pidfd_open, our_pid, 0) } as c_int);
507-
match pidfd {
501+
502+
match PidFd::current_process() {
508503
Ok(pidfd) => {
504+
// if pidfd_open works then we at least know the fork path is available.
509505
support = FORK_EXEC;
510-
if let Some(Ok(pid)) = pidfd_getpid.get().map(|f| cvt(unsafe { f(pidfd) } as i32)) {
511-
if pidfd_spawnp.get().is_some() && pid as u32 == our_pid {
512-
support = SPAWN
513-
}
506+
// but for the fast path we need both spawnp and the
507+
// pidfd -> pid conversion to work.
508+
if pidfd_spawnp.get().is_some() && let Ok(pid) = pidfd.pid() {
509+
assert_eq!(pid, crate::process::id(), "sanity check");
510+
support = SPAWN;
514511
}
515-
unsafe { libc::close(pidfd) };
516512
}
517513
Err(e) if e.raw_os_error() == Some(libc::EMFILE) => {
518-
// We're temporarily(?) out of file descriptors. In this case obtaining a pidfd would also fail
514+
// We're temporarily(?) out of file descriptors. In this case pidfd_spawnp would also fail
519515
// Don't update the support flag so we can probe again later.
520516
return Err(e)
521517
}
522-
_ => {}
518+
_ => {
519+
// pidfd_open not available? likely an old kernel without pidfd support.
520+
}
523521
}
524522
PIDFD_SUPPORTED.store(support, Ordering::Relaxed);
525523
if support == FORK_EXEC {
@@ -791,21 +789,25 @@ impl Command {
791789
}
792790
spawn_res?;
793791

794-
let pid = match cvt(pidfd_getpid.get().unwrap()(pidfd)) {
792+
use crate::os::fd::{FromRawFd, IntoRawFd};
793+
794+
let pidfd = PidFd::from_raw_fd(pidfd);
795+
let pid = match pidfd.pid() {
795796
Ok(pid) => pid,
796797
Err(e) => {
797798
// The child has been spawned and we are holding its pidfd.
798-
// But we cannot obtain its pid even though pidfd_getpid support was verified earlier.
799-
// This might happen if libc can't open procfs because the file descriptor limit has been reached.
800-
libc::close(pidfd);
799+
// But we cannot obtain its pid even though pidfd_spawnp and getpid support
800+
// was verified earlier.
801+
// This is quite unlikely, but might happen if the ioctl is not supported,
802+
// glibc tries to use procfs and we're out of file descriptors.
801803
return Err(Error::new(
802804
e.kind(),
803805
"pidfd_spawnp succeeded but the child's PID could not be obtained",
804806
));
805807
}
806808
};
807809

808-
return Ok(Some(Process::new(pid, pidfd)));
810+
return Ok(Some(Process::new(pid as i32, pidfd.into_raw_fd())));
809811
}
810812

811813
// Safety: -1 indicates we don't have a pidfd.

0 commit comments

Comments
 (0)