Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support setting signal masks for child processes on Unix #100737

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 115 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,107 @@ pub trait CommandExt: Sealed {
/// ```
#[stable(feature = "process_set_process_group", since = "1.64.0")]
fn process_group(&mut self, pgroup: i32) -> &mut process::Command;

/// Blocks the given signal for the child process at the time it is started.
///
/// The set of blocked signals for a process is known as its signal mask.
/// Use this method to block some signals.
///
/// This method corresponds to calling [`sigaddset`] with the given signal.
///
/// # Notes
///
/// Rust's current default is to not block any signals in child processes. This may change in
/// the future to inheriting the current process's signal mask.
///
/// This method is idempotent: blocking a signal that's already blocked results in
/// success and has no effect.
///
/// Blocking some signals like `SIGSEGV` can lead to undefined behavior in
/// the child. See the [`pthread_sigmask`] man page for more information.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sigprocmask is probably the more canonical reference.

Also, while the manpage may say that the effect of blocking synchronous traps (ie, SEGV/BUS/FPE/ILL raised while executing an instruction) is undefined, I wouldn't read that as being the same as UB at the Rust/llvm level.

In practice I think it would either deliver the signal as SIG_DFL (ie, Linux), or endlessly re-trap on the same instruction (what x86 macOS seems to do) - either way the program has effectively terminated and won't enter any UB states as far as the program semantics are concerned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh it's actually the same man page for both pthread_sigmask and sigprocmask. I can change the reference in the docs to be sigprocmask though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh - on Fedora they're separate, but pthread_sigmask has almost no content and refers to sigprocmask for discussion of the detailed semantics.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I was referring to the man pages from the official POSIX spec here, as linked in the comment)

///
/// # Errors
///
/// Returns an `InvalidInput` error if the signal is invalid.
///
/// # Examples
///
/// Start a process with `SIGINT` blocked:
///
/// ```no_run
/// #![feature(process_sigmask)]
/// #
/// use std::process::Command;
/// use std::os::unix::process::CommandExt;
///
/// Command::new("sleep")
/// .arg("10")
/// // On most platforms, SIGINT is signal 2.
/// .block_signal(2)?
/// .spawn()?;
/// #
/// # Ok::<_, Box<dyn std::error::Error>>(())
/// ```
///
/// [`sigaddset`]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigaddset.html
/// [`pthread_sigmask`]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_sigmask.html
#[unstable(feature = "process_sigmask", issue = "none")]
fn block_signal(&mut self, signal: i32) -> io::Result<&mut process::Command>;

/// Unblocks the given signal for the child process at the time it is started.
///
/// The set of blocked signals for a process is known as its signal mask.
/// Use this method to unblock a signal.
///
/// This method corresponds to calling [`sigdelset`] with the given signal.
///
/// # Notes
///
/// Rust's current default is to not block any signals in child processes. This may change in
/// the future to inheriting the current process's signal mask.
///
/// This method is idempotent: unblocking a signal that's already unblocked results in
/// success and has no effect.
///
/// # Errors
///
/// Returns an `InvalidInput` error if the signal is invalid.
///
/// # Examples
///
/// Start a process with `SIGHUP` unblocked:
///
/// ```no_run
/// #![feature(process_sigmask)]
/// #
/// use std::process::Command;
/// use std::os::unix::process::CommandExt;
///
/// Command::new("sleep")
/// .arg("10")
/// // On most platforms, SIGHUP is signal 1.
/// .unblock_signal(1)?
/// .spawn()?;
/// #
/// # Ok::<_, Box<dyn std::error::Error>>(())
/// ```
///
/// [`sigdelset`]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigdelset.html
/// [`pthread_sigmask`]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_sigmask.html
#[unstable(feature = "process_sigmask", issue = "none")]
fn unblock_signal(&mut self, signal: i32) -> io::Result<&mut process::Command>;

/// Returns true if a signal will be blocked in the child process at the time it is started.
///
/// This method corresponds to calling [`sigismember`] with the given signal.
///
/// # Errors
///
/// Returns an `InvalidInput` error if the signal is invalid.
///
/// [`sigismember`]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigismember.html
#[unstable(feature = "process_sigmask", issue = "none")]
fn will_block_signal(&self, signal: i32) -> io::Result<bool>;
}

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

fn block_signal(&mut self, signal: i32) -> io::Result<&mut process::Command> {
self.as_inner_mut().signal_mask()?.insert(signal)?;
Ok(self)
}

fn unblock_signal(&mut self, signal: i32) -> io::Result<&mut process::Command> {
self.as_inner_mut().signal_mask()?.remove(signal)?;
Ok(self)
}

fn will_block_signal(&self, signal: i32) -> io::Result<bool> {
self.as_inner().get_signal_mask()?.contains(signal)
}
}

/// Unix-specific extensions to [`process::ExitStatus`] and
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/unix/process/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
pub use self::process_common::{Command, CommandArgs, ExitCode, Stdio, StdioPipes};
pub use self::process_common::{Command, CommandArgs, ExitCode, SignalSet, Stdio, StdioPipes};
pub use self::process_inner::{ExitStatus, ExitStatusError, Process};
pub use crate::ffi::OsString as EnvKey;
pub use crate::sys_common::process::CommandEnvs;
Expand Down
122 changes: 113 additions & 9 deletions library/std/src/sys/unix/process/process_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ use crate::fmt;
use crate::io;
use crate::path::Path;
use crate::ptr;
use crate::sync::OnceLock;
use crate::sys::fd::FileDesc;
use crate::sys::fs::File;
use crate::sys::pipe::{self, AnonPipe};
use crate::sys::{cvt, cvt_nz};
use crate::sys_common::process::{CommandEnv, CommandEnvs};
use crate::sys_common::IntoInner;

Expand All @@ -32,9 +34,9 @@ cfg_if::cfg_if! {
}
}

// Android with api less than 21 define sig* functions inline, so it is not
// available for dynamic link. Implementing sigemptyset and sigaddset allow us
// to support older Android version (independent of libc version).
// Android with api less than 21 define sig* functions inline, so they are not
// available for dynamic linking. Implementing these functions allows us
// to support older Android versions (independent of libc version).
// The following implementations are based on
// https://github.com/aosp-mirror/platform_bionic/blob/ad8dcd6023294b646e5a8288c0ed431b0845da49/libc/include/android/legacy_signal_inlines.h
cfg_if::cfg_if! {
Expand All @@ -43,8 +45,58 @@ cfg_if::cfg_if! {
set.write_bytes(0u8, 1);
return 0;
}

const LONG_BIT: usize = crate::mem::size_of::<libc::c_ulong>() * 8;

#[allow(dead_code)]
pub unsafe fn sigaddset(set: *mut libc::sigset_t, signum: libc::c_int) -> libc::c_int {
let bit = (signum - 1) as usize;
let raw = match to_bitmap_slice_mut(set, bit) {
Ok(raw) => raw,
Err(val) => return val,
};
raw[bit / LONG_BIT] |= 1 << (bit % LONG_BIT);
return 0;
}

#[allow(dead_code)]
pub unsafe fn sigdelset(set: *mut libc::sigset_t, signum: libc::c_int) -> libc::c_int {
let bit = (signum - 1) as usize;
let raw = match to_bitmap_slice_mut(set, bit) {
Ok(raw) => raw,
Err(val) => return val,
};
raw[bit / LONG_BIT] &= !(1 << (bit % LONG_BIT));
return 0;
}

#[allow(dead_code)]
pub unsafe fn sigismember(set: *const libc::sigset_t, signum: libc::c_int) -> libc::c_int {
// Can't use to_bitmap_slice_mut because it's *mut, not *const.
use crate::{
mem::size_of,
slice,
};
use libc::{c_ulong, sigset_t};

let bit = (signum - 1) as usize;
if set.is_null() || bit >= (8 * size_of::<sigset_t>()) {
crate::sys::unix::os::set_errno(libc::EINVAL);
return -1;
}
let raw: &[c_ulong] = slice::from_raw_parts(
set as *const c_ulong,
size_of::<sigset_t>() / size_of::<c_ulong>(),
);

return ((raw[bit / LONG_BIT] >> (bit % LONG_BIT)) & 1) as i32;
}

// SAFETY: returned slice lives as long as set.
unsafe fn to_bitmap_slice_mut<'a>(
set: *mut libc::sigset_t,
bit: usize,
) -> Result<&'a mut [libc::c_ulong], libc::c_int> {
use crate::{
mem::{align_of, size_of},
slice,
Expand All @@ -59,21 +111,19 @@ cfg_if::cfg_if! {
&& (size_of::<sigset_t>() % size_of::<c_ulong>()) == 0
);

let bit = (signum - 1) as usize;
if set.is_null() || bit >= (8 * size_of::<sigset_t>()) {
crate::sys::unix::os::set_errno(libc::EINVAL);
return -1;
return Err(-1);
}
let raw = slice::from_raw_parts_mut(
set as *mut c_ulong,
size_of::<sigset_t>() / size_of::<c_ulong>(),
);
const LONG_BIT: usize = size_of::<c_ulong>() * 8;
raw[bit / LONG_BIT] |= 1 << (bit % LONG_BIT);
return 0;

Ok(raw)
}
} else {
pub use libc::{sigemptyset, sigaddset};
pub use libc::{sigemptyset, sigaddset, sigdelset, sigismember};
}
}

Expand Down Expand Up @@ -104,6 +154,7 @@ pub struct Command {
#[cfg(target_os = "linux")]
create_pidfd: bool,
pgroup: Option<pid_t>,
signal_mask: OnceLock<SignalSet>,
}

// Create a new type for argv, so that we can make it `Send` and `Sync`
Expand Down Expand Up @@ -148,6 +199,49 @@ pub enum Stdio {
Fd(FileDesc),
}

pub struct SignalSet {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be private?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It follows the pattern of the other items in the module which are all declared pub (though they actually resolve to pub(crate)).

pub sigset: libc::sigset_t,
}

impl SignalSet {
#[allow(dead_code)]
pub fn new_from_current() -> io::Result<Self> {
let mut set = crate::mem::MaybeUninit::<libc::sigset_t>::uninit();
// pthread_sigmask returns the errno rather than setting it directly.
unsafe {
cvt_nz(libc::pthread_sigmask(libc::SIG_BLOCK, ptr::null(), set.as_mut_ptr()))?;
Ok(Self { sigset: set.assume_init() })
}
}

pub fn empty() -> io::Result<Self> {
let mut set = crate::mem::MaybeUninit::<libc::sigset_t>::uninit();
unsafe {
cvt(sigemptyset(set.as_mut_ptr()))?;
Ok(Self { sigset: set.assume_init() })
}
}

pub fn insert(&mut self, signal: i32) -> io::Result<&mut Self> {
unsafe {
cvt(sigaddset(&mut self.sigset, signal))?;
}
Ok(self)
}

pub fn remove(&mut self, signal: i32) -> io::Result<&mut Self> {
unsafe {
cvt(sigdelset(&mut self.sigset, signal))?;
}
Ok(self)
}

pub fn contains(&self, signal: i32) -> io::Result<bool> {
let contains = unsafe { cvt(sigismember(&self.sigset, signal))? };
Ok(contains != 0)
}
}

impl Command {
#[cfg(not(target_os = "linux"))]
pub fn new(program: &OsStr) -> Command {
Expand All @@ -168,6 +262,7 @@ impl Command {
stdout: None,
stderr: None,
pgroup: None,
mask: None,
}
}

Expand All @@ -191,6 +286,7 @@ impl Command {
stderr: None,
create_pidfd: false,
pgroup: None,
signal_mask: OnceLock::new(),
}
}

Expand Down Expand Up @@ -229,6 +325,10 @@ impl Command {
pub fn pgroup(&mut self, pgroup: pid_t) {
self.pgroup = Some(pgroup);
}
pub fn signal_mask(&mut self) -> io::Result<&mut SignalSet> {
self.signal_mask.get_or_try_init(SignalSet::empty)?;
Ok(self.signal_mask.get_mut().unwrap())
}

#[cfg(target_os = "linux")]
pub fn create_pidfd(&mut self, val: bool) {
Expand Down Expand Up @@ -296,6 +396,10 @@ impl Command {
pub fn get_pgroup(&self) -> Option<pid_t> {
self.pgroup
}
#[allow(dead_code)]
pub fn get_signal_mask(&self) -> io::Result<&SignalSet> {
self.signal_mask.get_or_try_init(SignalSet::empty)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll want to remove the OnceLock from this if the other PR's FCP doesn't get accepted.

}

pub fn get_closures(&mut self) -> &mut Vec<Box<dyn FnMut() -> io::Result<()> + Send + Sync>> {
&mut self.closures
Expand Down
25 changes: 9 additions & 16 deletions library/std/src/sys/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,18 +326,10 @@ impl Command {
// emscripten has no signal support.
#[cfg(not(target_os = "emscripten"))]
{
use crate::mem::MaybeUninit;
use crate::sys::cvt_nz;
// Reset signal handling so the child process starts in a
// standardized state. libstd ignores SIGPIPE, and signal-handling
// libraries often set a mask. Child processes inherit ignored
// signals and the signal mask from their parent, but most
// UNIX programs do not reset these things on their own, so we
// need to clean things up now to avoid confusing the program
// we're about to run.
let mut set = MaybeUninit::<libc::sigset_t>::uninit();
cvt(sigemptyset(set.as_mut_ptr()))?;
cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, set.as_ptr(), ptr::null_mut()))?;
// Set the signal mask.
let signal_mask = self.get_signal_mask()?;
sunshowers marked this conversation as resolved.
Show resolved Hide resolved
cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, &signal_mask.sigset, ptr::null_mut()))?;

#[cfg(target_os = "android")] // see issue #88585
{
Expand Down Expand Up @@ -529,11 +521,12 @@ impl Command {
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 signal_mask = self.get_signal_mask()?;
sunshowers marked this conversation as resolved.
Show resolved Hide resolved
cvt_nz(libc::posix_spawnattr_setsigmask(attrs.0.as_mut_ptr(), &signal_mask.sigset))?;

let mut sig_default = SignalSet::empty()?;
sig_default.insert(libc::SIGPIPE)?;
cvt_nz(libc::posix_spawnattr_setsigdefault(attrs.0.as_mut_ptr(), &sig_default.sigset))?;

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