diff --git a/library/std/src/os/linux/process.rs b/library/std/src/os/linux/process.rs index fef321436f6a3..60851db831bf9 100644 --- a/library/std/src/os/linux/process.rs +++ b/library/std/src/os/linux/process.rs @@ -67,8 +67,10 @@ impl PidFd { /// Waits for the child to exit completely, returning the status that it exited with. /// /// Unlike [`Child::wait`] it does not ensure that the stdin handle is closed. - /// Additionally it will not return an `ExitStatus` if the child - /// has already been reaped. Instead an error will be returned. + /// + /// Additionally on kernels prior to 6.15 only the first attempt to + /// reap a child will return an ExitStatus, further attempts + /// will return an Error. /// /// [`Child::wait`]: process::Child::wait pub fn wait(&self) -> Result { @@ -77,8 +79,8 @@ impl PidFd { /// Attempts to collect the exit status of the child if it has already exited. /// - /// Unlike [`Child::try_wait`] this method will return an Error - /// if the child has already been reaped. + /// On kernels prior to 6.15, and unlike [`Child::try_wait`], only the first attempt + /// to reap a child will return an ExitStatus, further attempts will return an Error. /// /// [`Child::try_wait`]: process::Child::try_wait pub fn try_wait(&self) -> Result> { diff --git a/library/std/src/sys/pal/unix/linux/pidfd.rs b/library/std/src/sys/pal/unix/linux/pidfd.rs index 7859854e96b4b..e9e4831fcc02b 100644 --- a/library/std/src/sys/pal/unix/linux/pidfd.rs +++ b/library/std/src/sys/pal/unix/linux/pidfd.rs @@ -1,5 +1,5 @@ use crate::io; -use crate::os::fd::{AsRawFd, FromRawFd, RawFd}; +use crate::os::fd::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; use crate::sys::fd::FileDesc; use crate::sys::process::ExitStatus; use crate::sys::{AsInner, FromInner, IntoInner, cvt}; @@ -15,6 +15,73 @@ impl PidFd { self.send_signal(libc::SIGKILL) } + #[cfg(any(test, target_env = "gnu", target_env = "musl"))] + pub(crate) fn current_process() -> io::Result { + let pid = crate::process::id(); + let pidfd = cvt(unsafe { libc::syscall(libc::SYS_pidfd_open, pid, 0) })?; + Ok(unsafe { PidFd::from_raw_fd(pidfd as RawFd) }) + } + + #[cfg(any(test, target_env = "gnu", target_env = "musl"))] + pub(crate) fn pid(&self) -> io::Result { + use crate::sys::weak::weak; + + // since kernel 6.13 + // https://lore.kernel.org/all/20241010155401.2268522-1-luca.boccassi@gmail.com/ + let mut pidfd_info: libc::pidfd_info = unsafe { crate::mem::zeroed() }; + pidfd_info.mask = libc::PIDFD_INFO_PID as u64; + match cvt(unsafe { libc::ioctl(self.0.as_raw_fd(), libc::PIDFD_GET_INFO, &mut pidfd_info) }) + { + Ok(_) => {} + Err(e) if e.raw_os_error() == Some(libc::EINVAL) => { + // kernel doesn't support that ioctl, try the glibc helper that looks at procfs + weak!( + fn pidfd_getpid(pidfd: RawFd) -> libc::pid_t; + ); + if let Some(pidfd_getpid) = pidfd_getpid.get() { + let pid: libc::c_int = cvt(unsafe { pidfd_getpid(self.0.as_raw_fd()) })?; + return Ok(pid as u32); + } + return Err(e); + } + Err(e) => return Err(e), + } + + Ok(pidfd_info.pid) + } + + fn exit_for_reaped_child(&self) -> io::Result { + // since kernel 6.15 + // https://lore.kernel.org/linux-fsdevel/20250305-work-pidfs-kill_on_last_close-v3-0-c8c3d8361705@kernel.org/T/ + let mut pidfd_info: libc::pidfd_info = unsafe { crate::mem::zeroed() }; + pidfd_info.mask = libc::PIDFD_INFO_EXIT as u64; + cvt(unsafe { libc::ioctl(self.0.as_raw_fd(), libc::PIDFD_GET_INFO, &mut pidfd_info) })?; + Ok(ExitStatus::new(pidfd_info.exit_code)) + } + + fn waitid(&self, options: libc::c_int) -> io::Result> { + let mut siginfo: libc::siginfo_t = unsafe { crate::mem::zeroed() }; + let r = cvt(unsafe { + libc::waitid(libc::P_PIDFD, self.0.as_raw_fd() as u32, &mut siginfo, options) + }); + match r { + Err(waitid_err) if waitid_err.raw_os_error() == Some(libc::ECHILD) => { + // already reaped + match self.exit_for_reaped_child() { + Ok(exit_status) => return Ok(Some(exit_status)), + Err(_) => return Err(waitid_err), + } + } + Err(e) => return Err(e), + Ok(_) => {} + } + if unsafe { siginfo.si_pid() } == 0 { + Ok(None) + } else { + Ok(Some(ExitStatus::from_waitid_siginfo(siginfo))) + } + } + pub(crate) fn send_signal(&self, signal: i32) -> io::Result<()> { cvt(unsafe { libc::syscall( @@ -29,29 +96,15 @@ impl PidFd { } pub fn wait(&self) -> io::Result { - let mut siginfo: libc::siginfo_t = unsafe { crate::mem::zeroed() }; - cvt(unsafe { - libc::waitid(libc::P_PIDFD, self.0.as_raw_fd() as u32, &mut siginfo, libc::WEXITED) - })?; - Ok(ExitStatus::from_waitid_siginfo(siginfo)) + let r = self.waitid(libc::WEXITED)?; + match r { + Some(exit_status) => Ok(exit_status), + None => unreachable!("waitid with WEXITED should not return None"), + } } pub fn try_wait(&self) -> io::Result> { - let mut siginfo: libc::siginfo_t = unsafe { crate::mem::zeroed() }; - - cvt(unsafe { - libc::waitid( - libc::P_PIDFD, - self.0.as_raw_fd() as u32, - &mut siginfo, - libc::WEXITED | libc::WNOHANG, - ) - })?; - if unsafe { siginfo.si_pid() } == 0 { - Ok(None) - } else { - Ok(Some(ExitStatus::from_waitid_siginfo(siginfo))) - } + self.waitid(libc::WEXITED | libc::WNOHANG) } } @@ -78,3 +131,9 @@ impl FromRawFd for PidFd { Self(FileDesc::from_raw_fd(fd)) } } + +impl IntoRawFd for PidFd { + fn into_raw_fd(self) -> RawFd { + self.0.into_raw_fd() + } +} diff --git a/library/std/src/sys/pal/unix/linux/pidfd/tests.rs b/library/std/src/sys/pal/unix/linux/pidfd/tests.rs index 17b06bea91278..a3bb5d5d64ba5 100644 --- a/library/std/src/sys/pal/unix/linux/pidfd/tests.rs +++ b/library/std/src/sys/pal/unix/linux/pidfd/tests.rs @@ -1,8 +1,11 @@ +use super::PidFd as InternalPidFd; use crate::assert_matches::assert_matches; -use crate::os::fd::{AsRawFd, RawFd}; +use crate::io::ErrorKind; +use crate::os::fd::AsRawFd; use crate::os::linux::process::{ChildExt, CommandExt as _}; use crate::os::unix::process::{CommandExt as _, ExitStatusExt}; use crate::process::Command; +use crate::sys::AsInner; #[test] fn test_command_pidfd() { @@ -48,11 +51,22 @@ fn test_command_pidfd() { let mut cmd = Command::new("false"); let mut child = unsafe { cmd.pre_exec(|| Ok(())) }.create_pidfd(true).spawn().unwrap(); - assert!(child.id() > 0 && child.id() < -1i32 as u32); + let id = child.id(); + + assert!(id > 0 && id < -1i32 as u32, "spawning with pidfd still returns a sane pid"); if pidfd_open_available { assert!(child.pidfd().is_ok()) } + + if let Ok(pidfd) = child.pidfd() { + match pidfd.as_inner().pid() { + Ok(pid) => assert_eq!(pid, id), + Err(e) if e.kind() == ErrorKind::InvalidInput => { /* older kernel */ } + Err(e) => panic!("unexpected error getting pid from pidfd: {}", e), + } + } + child.wait().expect("error waiting on child"); } @@ -77,9 +91,15 @@ fn test_pidfd() { assert_eq!(status.signal(), Some(libc::SIGKILL)); // Trying to wait again for a reaped child is safe since there's no pid-recycling race. - // But doing so will return an error. + // But doing so may return an error. let res = fd.wait(); - assert_matches!(res, Err(e) if e.raw_os_error() == Some(libc::ECHILD)); + match res { + // older kernels + Err(e) if e.raw_os_error() == Some(libc::ECHILD) => {} + // 6.15+ + Ok(exit) if exit.signal() == Some(libc::SIGKILL) => {} + other => panic!("expected ECHILD error, got {:?}", other), + } // Ditto for additional attempts to kill an already-dead child. let res = fd.kill(); @@ -87,13 +107,5 @@ fn test_pidfd() { } fn probe_pidfd_support() -> bool { - // pidfds require the pidfd_open syscall - let our_pid = crate::process::id(); - let pidfd = unsafe { libc::syscall(libc::SYS_pidfd_open, our_pid, 0) }; - if pidfd >= 0 { - unsafe { libc::close(pidfd as RawFd) }; - true - } else { - false - } + InternalPidFd::current_process().is_ok() } diff --git a/library/std/src/sys/process/unix/unix.rs b/library/std/src/sys/process/unix/unix.rs index 5ba57e11679cf..df64a1716d523 100644 --- a/library/std/src/sys/process/unix/unix.rs +++ b/library/std/src/sys/process/unix/unix.rs @@ -482,10 +482,6 @@ impl Command { ) -> libc::c_int; ); - weak!( - fn pidfd_getpid(pidfd: libc::c_int) -> libc::c_int; - ); - static PIDFD_SUPPORTED: Atomic = AtomicU8::new(0); const UNKNOWN: u8 = 0; const SPAWN: u8 = 1; @@ -502,24 +498,26 @@ impl Command { } if support == UNKNOWN { support = NO; - let our_pid = crate::process::id(); - let pidfd = cvt(unsafe { libc::syscall(libc::SYS_pidfd_open, our_pid, 0) } as c_int); - match pidfd { + + match PidFd::current_process() { Ok(pidfd) => { + // if pidfd_open works then we at least know the fork path is available. support = FORK_EXEC; - if let Some(Ok(pid)) = pidfd_getpid.get().map(|f| cvt(unsafe { f(pidfd) } as i32)) { - if pidfd_spawnp.get().is_some() && pid as u32 == our_pid { - support = SPAWN - } + // but for the fast path we need both spawnp and the + // pidfd -> pid conversion to work. + if pidfd_spawnp.get().is_some() && let Ok(pid) = pidfd.pid() { + assert_eq!(pid, crate::process::id(), "sanity check"); + support = SPAWN; } - unsafe { libc::close(pidfd) }; } Err(e) if e.raw_os_error() == Some(libc::EMFILE) => { - // We're temporarily(?) out of file descriptors. In this case obtaining a pidfd would also fail + // We're temporarily(?) out of file descriptors. In this case pidfd_spawnp would also fail // Don't update the support flag so we can probe again later. return Err(e) } - _ => {} + _ => { + // pidfd_open not available? likely an old kernel without pidfd support. + } } PIDFD_SUPPORTED.store(support, Ordering::Relaxed); if support == FORK_EXEC { @@ -791,13 +789,17 @@ impl Command { } spawn_res?; - let pid = match cvt(pidfd_getpid.get().unwrap()(pidfd)) { + use crate::os::fd::{FromRawFd, IntoRawFd}; + + let pidfd = PidFd::from_raw_fd(pidfd); + let pid = match pidfd.pid() { Ok(pid) => pid, Err(e) => { // The child has been spawned and we are holding its pidfd. - // But we cannot obtain its pid even though pidfd_getpid support was verified earlier. - // This might happen if libc can't open procfs because the file descriptor limit has been reached. - libc::close(pidfd); + // But we cannot obtain its pid even though pidfd_spawnp and getpid support + // was verified earlier. + // This is quite unlikely, but might happen if the ioctl is not supported, + // glibc tries to use procfs and we're out of file descriptors. return Err(Error::new( e.kind(), "pidfd_spawnp succeeded but the child's PID could not be obtained", @@ -805,7 +807,7 @@ impl Command { } }; - return Ok(Some(Process::new(pid, pidfd))); + return Ok(Some(Process::new(pid as i32, pidfd.into_raw_fd()))); } // Safety: -1 indicates we don't have a pidfd.