Skip to content

Commit

Permalink
Rollup merge of rust-lang#93858 - krallin:process-process_group, r=dt…
Browse files Browse the repository at this point in the history
…olnay

Add a `process_group` method to UNIX `CommandExt`

- Tracking issue: rust-lang#93857
- RFC: rust-lang/rfcs#3228

Add a `process_group` method to `std::os::unix::process::CommandExt` that
allows setting the process group id (i.e. calling `setpgid`) in the child, thus
enabling users to set process groups while leveraging the `posix_spawn` fast
path.
  • Loading branch information
Dylan-DPC authored Mar 19, 2022
2 parents 3153584 + b628497 commit 3545003
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 2 deletions.
10 changes: 10 additions & 0 deletions library/std/src/os/unix/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,11 @@ pub trait CommandExt: Sealed {
fn arg0<S>(&mut self, arg: S) -> &mut process::Command
where
S: AsRef<OsStr>;

/// Sets the process group ID of the child process. Translates to a `setpgid` call in the child
/// process.
#[unstable(feature = "process_set_process_group", issue = "93857")]
fn process_group(&mut self, pgroup: i32) -> &mut process::Command;
}

#[stable(feature = "rust1", since = "1.0.0")]
Expand Down Expand Up @@ -201,6 +206,11 @@ impl CommandExt for process::Command {
self.as_inner_mut().set_arg_0(arg.as_ref());
self
}

fn process_group(&mut self, pgroup: i32) -> &mut process::Command {
self.as_inner_mut().pgroup(pgroup);
self
}
}

/// Unix-specific extensions to [`process::ExitStatus`] and
Expand Down
12 changes: 11 additions & 1 deletion library/std/src/sys/unix/process/process_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::sys_common::IntoInner;
#[cfg(not(target_os = "fuchsia"))]
use crate::sys::fs::OpenOptions;

use libc::{c_char, c_int, gid_t, uid_t, EXIT_FAILURE, EXIT_SUCCESS};
use libc::{c_char, c_int, gid_t, pid_t, uid_t, EXIT_FAILURE, EXIT_SUCCESS};

cfg_if::cfg_if! {
if #[cfg(target_os = "fuchsia")] {
Expand Down Expand Up @@ -82,6 +82,7 @@ pub struct Command {
stderr: Option<Stdio>,
#[cfg(target_os = "linux")]
create_pidfd: bool,
pgroup: Option<pid_t>,
}

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

Expand All @@ -167,6 +169,7 @@ impl Command {
stdout: None,
stderr: None,
create_pidfd: false,
pgroup: None,
}
}

Expand Down Expand Up @@ -202,6 +205,9 @@ impl Command {
pub fn groups(&mut self, groups: &[gid_t]) {
self.groups = Some(Box::from(groups));
}
pub fn pgroup(&mut self, pgroup: pid_t) {
self.pgroup = Some(pgroup);
}

#[cfg(target_os = "linux")]
pub fn create_pidfd(&mut self, val: bool) {
Expand Down Expand Up @@ -265,6 +271,10 @@ impl Command {
pub fn get_groups(&self) -> Option<&[gid_t]> {
self.groups.as_deref()
}
#[allow(dead_code)]
pub fn get_pgroup(&self) -> Option<pid_t> {
self.pgroup
}

pub fn get_closures(&mut self) -> &mut Vec<Box<dyn FnMut() -> io::Result<()> + Send + Sync>> {
&mut self.closures
Expand Down
55 changes: 55 additions & 0 deletions library/std/src/sys/unix/process/process_common/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,58 @@ fn test_process_mask() {
t!(cat.wait());
}
}

#[test]
#[cfg_attr(
any(
// See test_process_mask
target_os = "macos",
target_arch = "arm",
target_arch = "aarch64",
target_arch = "riscv64",
),
ignore
)]
fn test_process_group_posix_spawn() {
unsafe {
// Spawn a cat subprocess that's just going to hang since there is no I/O.
let mut cmd = Command::new(OsStr::new("cat"));
cmd.pgroup(0);
cmd.stdin(Stdio::MakePipe);
cmd.stdout(Stdio::MakePipe);
let (mut cat, _pipes) = t!(cmd.spawn(Stdio::Null, true));

// Check that we can kill its process group, which means there *is* one.
t!(cvt(libc::kill(-(cat.id() as libc::pid_t), libc::SIGINT)));

t!(cat.wait());
}
}

#[test]
#[cfg_attr(
any(
// See test_process_mask
target_os = "macos",
target_arch = "arm",
target_arch = "aarch64",
target_arch = "riscv64",
),
ignore
)]
fn test_process_group_no_posix_spawn() {
unsafe {
// Same as above, create hang-y cat. This time, force using the non-posix_spawnp path.
let mut cmd = Command::new(OsStr::new("cat"));
cmd.pgroup(0);
cmd.pre_exec(Box::new(|| Ok(()))); // pre_exec forces fork + exec
cmd.stdin(Stdio::MakePipe);
cmd.stdout(Stdio::MakePipe);
let (mut cat, _pipes) = t!(cmd.spawn(Stdio::Null, true));

// Check that we can kill its process group, which means there *is* one.
t!(cvt(libc::kill(-(cat.id() as libc::pid_t), libc::SIGINT)));

t!(cat.wait());
}
}
15 changes: 14 additions & 1 deletion library/std/src/sys/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,10 @@ impl Command {
cvt(libc::chdir(cwd.as_ptr()))?;
}

if let Some(pgroup) = self.get_pgroup() {
cvt(libc::setpgid(0, pgroup))?;
}

// emscripten has no signal support.
#[cfg(not(target_os = "emscripten"))]
{
Expand Down Expand Up @@ -459,6 +463,8 @@ impl Command {
None => None,
};

let pgroup = self.get_pgroup();

// Safety: -1 indicates we don't have a pidfd.
let mut p = unsafe { Process::new(0, -1) };

Expand Down Expand Up @@ -487,6 +493,8 @@ impl Command {
cvt_nz(libc::posix_spawnattr_init(attrs.as_mut_ptr()))?;
let attrs = PosixSpawnattr(&mut attrs);

let mut flags = 0;

let mut file_actions = MaybeUninit::uninit();
cvt_nz(libc::posix_spawn_file_actions_init(file_actions.as_mut_ptr()))?;
let file_actions = PosixSpawnFileActions(&mut file_actions);
Expand Down Expand Up @@ -516,13 +524,18 @@ impl Command {
cvt_nz(f(file_actions.0.as_mut_ptr(), cwd.as_ptr()))?;
}

if let Some(pgroup) = pgroup {
flags |= libc::POSIX_SPAWN_SETPGROUP;
cvt_nz(libc::posix_spawnattr_setpgroup(attrs.0.as_mut_ptr(), pgroup))?;
}

let mut set = MaybeUninit::<libc::sigset_t>::uninit();
cvt(sigemptyset(set.as_mut_ptr()))?;
cvt_nz(libc::posix_spawnattr_setsigmask(attrs.0.as_mut_ptr(), set.as_ptr()))?;
cvt(sigaddset(set.as_mut_ptr(), libc::SIGPIPE))?;
cvt_nz(libc::posix_spawnattr_setsigdefault(attrs.0.as_mut_ptr(), set.as_ptr()))?;

let flags = libc::POSIX_SPAWN_SETSIGDEF | libc::POSIX_SPAWN_SETSIGMASK;
flags |= libc::POSIX_SPAWN_SETSIGDEF | libc::POSIX_SPAWN_SETSIGMASK;
cvt_nz(libc::posix_spawnattr_setflags(attrs.0.as_mut_ptr(), flags as _))?;

// Make sure we synchronize access to the global `environ` resource
Expand Down

0 comments on commit 3545003

Please sign in to comment.