Skip to content

Commit

Permalink
rust: library: Add setsid method to CommandExt trait
Browse files Browse the repository at this point in the history
Add a setsid method to the CommandExt trait so that callers can create
a process in a new session and process group whilst still using the
POSIX spawn fast path.

Tracking issue: #105376
  • Loading branch information
HarveyHunt committed Jan 11, 2023
1 parent b22c152 commit 231d19f
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 0 deletions.
8 changes: 8 additions & 0 deletions library/std/src/os/unix/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ pub trait CommandExt: Sealed {
/// ```
#[stable(feature = "process_set_process_group", since = "1.64.0")]
fn process_group(&mut self, pgroup: i32) -> &mut process::Command;

#[unstable(feature = "process_setsid", issue = "105376")]
fn setsid(&mut self) -> &mut process::Command;
}

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

fn setsid(&mut self) -> &mut process::Command {
self.as_inner_mut().setsid();
self
}
}

/// Unix-specific extensions to [`process::ExitStatus`] and
Expand Down
12 changes: 12 additions & 0 deletions library/std/src/sys/unix/process/process_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ pub struct Command {
#[cfg(target_os = "linux")]
create_pidfd: bool,
pgroup: Option<pid_t>,
setsid: bool,
}

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

Expand All @@ -222,6 +224,7 @@ impl Command {
stderr: None,
create_pidfd: false,
pgroup: None,
setsid: false,
}
}

Expand Down Expand Up @@ -261,6 +264,10 @@ impl Command {
self.pgroup = Some(pgroup);
}

pub fn setsid(&mut self) {
self.setsid = true;
}

#[cfg(target_os = "linux")]
pub fn create_pidfd(&mut self, val: bool) {
self.create_pidfd = val;
Expand Down Expand Up @@ -333,6 +340,11 @@ impl Command {
self.pgroup
}

#[allow(dead_code)]
pub fn get_setsid(&self) -> bool {
self.setsid
}

pub fn get_closures(&mut self) -> &mut Vec<Box<dyn FnMut() -> io::Result<()> + Send + Sync>> {
&mut self.closures
}
Expand Down
58 changes: 58 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 @@ -136,6 +136,64 @@ fn test_process_group_no_posix_spawn() {
}
}

#[test]
#[cfg_attr(
any(
// See test_process_mask
target_os = "macos",
target_arch = "arm",
target_arch = "aarch64",
target_arch = "riscv64",
),
ignore
)]
fn test_setsid_posix_spawn() {
// 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.setsid();
cmd.stdin(Stdio::MakePipe);
cmd.stdout(Stdio::MakePipe);
let (mut cat, _pipes) = t!(cmd.spawn(Stdio::Null, true));

unsafe {
// Setsid will create a new session and process group, so check that
// we can kill the 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_setsid_no_posix_spawn() {
let mut cmd = Command::new(OsStr::new("cat"));
cmd.setsid();
cmd.stdin(Stdio::MakePipe);
cmd.stdout(Stdio::MakePipe);

unsafe {
// Same as above, create hang-y cat. This time, force using the non-posix_spawn path.
cmd.pre_exec(Box::new(|| Ok(()))); // pre_exec forces fork + exec rather than posix spawn.
let (mut cat, _pipes) = t!(cmd.spawn(Stdio::Null, true));

// Setsid will create a new session and process group, so check that
// we can kill the process group, which means there *is* one.
t!(cvt(libc::kill(-(cat.id() as libc::pid_t), libc::SIGINT)));

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

#[test]
fn test_program_kind() {
let vectors = &[
Expand Down
9 changes: 9 additions & 0 deletions library/std/src/sys/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,10 @@ impl Command {
cvt(libc::setpgid(0, pgroup))?;
}

if self.get_setsid() {
cvt(libc::setsid())?;
}

// emscripten has no signal support.
#[cfg(not(target_os = "emscripten"))]
{
Expand Down Expand Up @@ -467,6 +471,7 @@ impl Command {
};

let pgroup = self.get_pgroup();
let setsid = self.get_setsid();

// Safety: -1 indicates we don't have a pidfd.
let mut p = unsafe { Process::new(0, -1) };
Expand Down Expand Up @@ -550,6 +555,10 @@ impl Command {
flags |= libc::POSIX_SPAWN_SETSIGDEF;
}

if setsid {
flags |= libc::POSIX_SPAWN_SETSID;
}

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 231d19f

Please sign in to comment.