Skip to content

Commit

Permalink
Rollup merge of rust-lang#81825 - voidc:pidfd, r=joshtriplett
Browse files Browse the repository at this point in the history
Add Linux-specific pidfd process extensions (take 2)

Continuation of rust-lang#77168.
I addressed the following concerns from the original PR:

- make `CommandExt` and `ChildExt` sealed traits
- wrap file descriptors in `PidFd` struct representing ownership over the fd
- add `take_pidfd` to take the fd out of `Child`
- close fd when dropped

Tracking Issue: rust-lang#82971
  • Loading branch information
Dylan-DPC authored Mar 18, 2021
2 parents c28d8d2 + cfb2d72 commit 717b6c4
Show file tree
Hide file tree
Showing 8 changed files with 318 additions and 7 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1895,9 +1895,9 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55"

[[package]]
name = "libc"
version = "0.2.88"
version = "0.2.89"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "03b07a082330a35e43f63177cc01689da34fbffa0105e1246cf0311472cac73a"
checksum = "538c092e5586f4cdd7dd8078c4a79220e3e168880218124dcbce860f0ea938c6"
dependencies = [
"rustc-std-workspace-core",
]
Expand Down
2 changes: 1 addition & 1 deletion library/std/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ cfg-if = { version = "0.1.8", features = ['rustc-dep-of-std'] }
panic_unwind = { path = "../panic_unwind", optional = true }
panic_abort = { path = "../panic_abort" }
core = { path = "../core" }
libc = { version = "0.2.88", default-features = false, features = ['rustc-dep-of-std'] }
libc = { version = "0.2.89", default-features = false, features = ['rustc-dep-of-std'] }
compiler_builtins = { version = "0.1.39" }
profiler_builtins = { path = "../profiler_builtins", optional = true }
unwind = { path = "../unwind" }
Expand Down
1 change: 1 addition & 0 deletions library/std/src/os/linux/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@
#![stable(feature = "raw_ext", since = "1.1.0")]

pub mod fs;
pub mod process;
pub mod raw;
150 changes: 150 additions & 0 deletions library/std/src/os/linux/process.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
//! Linux-specific extensions to primitives in the `std::process` module.

#![unstable(feature = "linux_pidfd", issue = "82971")]

use crate::io::Result;
use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, RawFd};
use crate::process;
use crate::sys::fd::FileDesc;
use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner};

/// This type represents a file descriptor that refers to a process.
///
/// A `PidFd` can be obtained by setting the corresponding option on [`Command`]
/// with [`create_pidfd`]. Subsequently, the created pidfd can be retrieved
/// from the [`Child`] by calling [`pidfd`] or [`take_pidfd`].
///
/// Example:
/// ```no_run
/// #![feature(linux_pidfd)]
/// use std::os::linux::process::{CommandExt, ChildExt};
/// use std::process::Command;
///
/// let mut child = Command::new("echo")
/// .create_pidfd(true)
/// .spawn()
/// .expect("Failed to spawn child");
///
/// let pidfd = child
/// .take_pidfd()
/// .expect("Failed to retrieve pidfd");
///
/// // The file descriptor will be closed when `pidfd` is dropped.
/// ```
/// Refer to the man page of [`pidfd_open(2)`] for further details.
///
/// [`Command`]: process::Command
/// [`create_pidfd`]: CommandExt::create_pidfd
/// [`Child`]: process::Child
/// [`pidfd`]: fn@ChildExt::pidfd
/// [`take_pidfd`]: ChildExt::take_pidfd
/// [`pidfd_open(2)`]: https://man7.org/linux/man-pages/man2/pidfd_open.2.html
#[derive(Debug)]
pub struct PidFd {
inner: FileDesc,
}

impl AsInner<FileDesc> for PidFd {
fn as_inner(&self) -> &FileDesc {
&self.inner
}
}

impl FromInner<FileDesc> for PidFd {
fn from_inner(inner: FileDesc) -> PidFd {
PidFd { inner }
}
}

impl IntoInner<FileDesc> for PidFd {
fn into_inner(self) -> FileDesc {
self.inner
}
}

impl AsRawFd for PidFd {
fn as_raw_fd(&self) -> RawFd {
self.as_inner().raw()
}
}

impl FromRawFd for PidFd {
unsafe fn from_raw_fd(fd: RawFd) -> Self {
Self::from_inner(FileDesc::new(fd))
}
}

impl IntoRawFd for PidFd {
fn into_raw_fd(self) -> RawFd {
self.into_inner().into_raw()
}
}

mod private_child_ext {
pub trait Sealed {}
impl Sealed for crate::process::Child {}
}

/// Os-specific extensions for [`Child`]
///
/// [`Child`]: process::Child
pub trait ChildExt: private_child_ext::Sealed {
/// Obtains a reference to the [`PidFd`] created for this [`Child`], if available.
///
/// A pidfd will only be available if its creation was requested with
/// [`create_pidfd`] when the corresponding [`Command`] was created.
///
/// Even if requested, a pidfd may not be available due to an older
/// version of Linux being in use, or if some other error occurred.
///
/// [`Command`]: process::Command
/// [`create_pidfd`]: CommandExt::create_pidfd
/// [`Child`]: process::Child
fn pidfd(&self) -> Result<&PidFd>;

/// Takes ownership of the [`PidFd`] created for this [`Child`], if available.
///
/// A pidfd will only be available if its creation was requested with
/// [`create_pidfd`] when the corresponding [`Command`] was created.
///
/// Even if requested, a pidfd may not be available due to an older
/// version of Linux being in use, or if some other error occurred.
///
/// [`Command`]: process::Command
/// [`create_pidfd`]: CommandExt::create_pidfd
/// [`Child`]: process::Child
fn take_pidfd(&mut self) -> Result<PidFd>;
}

mod private_command_ext {
pub trait Sealed {}
impl Sealed for crate::process::Command {}
}

/// Os-specific extensions for [`Command`]
///
/// [`Command`]: process::Command
pub trait CommandExt: private_command_ext::Sealed {
/// Sets whether a [`PidFd`](struct@PidFd) should be created for the [`Child`]
/// spawned by this [`Command`].
/// By default, no pidfd will be created.
///
/// The pidfd can be retrieved from the child with [`pidfd`] or [`take_pidfd`].
///
/// A pidfd will only be created if it is possible to do so
/// in a guaranteed race-free manner (e.g. if the `clone3` system call
/// is supported). Otherwise, [`pidfd`] will return an error.
///
/// [`Command`]: process::Command
/// [`Child`]: process::Child
/// [`pidfd`]: fn@ChildExt::pidfd
/// [`take_pidfd`]: ChildExt::take_pidfd
fn create_pidfd(&mut self, val: bool) -> &mut process::Command;
}

impl CommandExt for process::Command {
fn create_pidfd(&mut self, val: bool) -> &mut process::Command {
self.as_inner_mut().create_pidfd(val);
self
}
}
2 changes: 1 addition & 1 deletion library/std/src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner};
/// [`wait`]: Child::wait
#[stable(feature = "process", since = "1.0.0")]
pub struct Child {
handle: imp::Process,
pub(crate) handle: imp::Process,

/// The handle for writing to the child's standard input (stdin), if it has
/// been captured. To avoid partially moving
Expand Down
6 changes: 6 additions & 0 deletions library/std/src/sys/unix/process/process_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ pub struct Command {
stdin: Option<Stdio>,
stdout: Option<Stdio>,
stderr: Option<Stdio>,
pub(crate) create_pidfd: bool,
}

// Create a new type for argv, so that we can make it `Send` and `Sync`
Expand Down Expand Up @@ -141,6 +142,7 @@ impl Command {
stdin: None,
stdout: None,
stderr: None,
create_pidfd: false,
}
}

Expand Down Expand Up @@ -177,6 +179,10 @@ impl Command {
self.groups = Some(Box::from(groups));
}

pub fn create_pidfd(&mut self, val: bool) {
self.create_pidfd = val;
}

pub fn saw_nul(&self) -> bool {
self.saw_nul
}
Expand Down
Loading

0 comments on commit 717b6c4

Please sign in to comment.