Change process spawning to inherit the parent's signal mask by default

Previously, the signal mask is always reset when a child process is
started. This breaks tools like `nohup` which expect `SIGHUP` to be
blocked.

With this change, the default behavior changes to inherit the signal mask.

This also changes the signal disposition for `SIGPIPE` to only be
changed if the `#[unix_sigpipe]` attribute isn't set.
This commit is contained in:
Rain 2022-09-22 22:48:14 -07:00
parent 542febd2d3
commit a52c79e859
8 changed files with 136 additions and 72 deletions

View File

@ -1,5 +1,13 @@
//! NOTE: Keep these constants in sync with `library/std/src/sys/unix/mod.rs`!
/// The default value if `#[unix_sigpipe]` is not specified. This resolves
/// to `SIG_IGN` in `library/std/src/sys/unix/mod.rs`.
///
/// Note that `SIG_IGN` has been the Rust default since 2014. See
/// <https://github.com/rust-lang/rust/issues/62569>.
#[allow(dead_code)]
pub const DEFAULT: u8 = 0;
/// Do not touch `SIGPIPE`. Use whatever the parent process uses.
#[allow(dead_code)]
pub const INHERIT: u8 = 1;
@ -15,8 +23,3 @@ pub const SIG_IGN: u8 = 2;
/// such as `head -n 1`.
#[allow(dead_code)]
pub const SIG_DFL: u8 = 3;
/// `SIG_IGN` has been the Rust default since 2014. See
/// <https://github.com/rust-lang/rust/issues/62569>.
#[allow(dead_code)]
pub const DEFAULT: u8 = SIG_IGN;

View File

@ -89,7 +89,7 @@ macro_rules! rtunwrap {
// `src/tools/tidy/src/pal.rs` for more info. On all other platforms, `sigpipe`
// has a value, but its value is ignored.
//
// Even though it is an `u8`, it only ever has 3 values. These are documented in
// Even though it is an `u8`, it only ever has 4 values. These are documented in
// `compiler/rustc_session/src/config/sigpipe.rs`.
#[cfg_attr(test, allow(dead_code))]
unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {

View File

@ -163,17 +163,27 @@ pub unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {
// See the other file for docs. NOTE: Make sure to keep them in
// sync!
mod sigpipe {
pub const DEFAULT: u8 = 0;
pub const INHERIT: u8 = 1;
pub const SIG_IGN: u8 = 2;
pub const SIG_DFL: u8 = 3;
}
let handler = match sigpipe {
sigpipe::INHERIT => None,
sigpipe::SIG_IGN => Some(libc::SIG_IGN),
sigpipe::SIG_DFL => Some(libc::SIG_DFL),
let (sigpipe_attr_specified, handler) = match sigpipe {
sigpipe::DEFAULT => (false, Some(libc::SIG_IGN)),
sigpipe::INHERIT => (true, None),
sigpipe::SIG_IGN => (true, Some(libc::SIG_IGN)),
sigpipe::SIG_DFL => (true, Some(libc::SIG_DFL)),
_ => unreachable!(),
};
// The bootstrap compiler doesn't know about sigpipe::DEFAULT, and always passes in
// SIG_IGN. This causes some tests to fail because they expect SIGPIPE to be reset to
// default on process spawning (which doesn't happen if #[unix_sigpipe] is specified).
// Since we can't differentiate between the cases here, treat SIG_IGN as DEFAULT
// unconditionally.
if sigpipe_attr_specified && !(cfg!(bootstrap) && sigpipe == sigpipe::SIG_IGN) {
UNIX_SIGPIPE_ATTR_SPECIFIED.store(true, crate::sync::atomic::Ordering::Relaxed);
}
if let Some(handler) = handler {
rtassert!(signal(libc::SIGPIPE, handler) != libc::SIG_ERR);
}
@ -181,6 +191,26 @@ pub unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {
}
}
// This is set (up to once) in reset_sigpipe.
#[cfg(not(any(
target_os = "espidf",
target_os = "emscripten",
target_os = "fuchsia",
target_os = "horizon"
)))]
static UNIX_SIGPIPE_ATTR_SPECIFIED: crate::sync::atomic::AtomicBool =
crate::sync::atomic::AtomicBool::new(false);
#[cfg(not(any(
target_os = "espidf",
target_os = "emscripten",
target_os = "fuchsia",
target_os = "horizon"
)))]
pub(crate) fn unix_sigpipe_attr_specified() -> bool {
UNIX_SIGPIPE_ATTR_SPECIFIED.load(crate::sync::atomic::Ordering::Relaxed)
}
// SAFETY: must be called only once during runtime cleanup.
// NOTE: this is not guaranteed to run, for example when the program aborts.
pub unsafe fn cleanup() {

View File

@ -39,10 +39,12 @@ cfg_if::cfg_if! {
// https://github.com/aosp-mirror/platform_bionic/blob/ad8dcd6023294b646e5a8288c0ed431b0845da49/libc/include/android/legacy_signal_inlines.h
cfg_if::cfg_if! {
if #[cfg(target_os = "android")] {
#[allow(dead_code)]
pub unsafe fn sigemptyset(set: *mut libc::sigset_t) -> libc::c_int {
set.write_bytes(0u8, 1);
return 0;
}
#[allow(dead_code)]
pub unsafe fn sigaddset(set: *mut libc::sigset_t, signum: libc::c_int) -> libc::c_int {
use crate::{

View File

@ -31,41 +31,54 @@ macro_rules! t {
ignore
)]
fn test_process_mask() {
unsafe {
// Test to make sure that a signal mask does not get inherited.
let mut cmd = Command::new(OsStr::new("cat"));
// Test to make sure that a signal mask *does* get inherited.
fn test_inner(mut cmd: Command) {
unsafe {
let mut set = mem::MaybeUninit::<libc::sigset_t>::uninit();
let mut old_set = mem::MaybeUninit::<libc::sigset_t>::uninit();
t!(cvt(sigemptyset(set.as_mut_ptr())));
t!(cvt(sigaddset(set.as_mut_ptr(), libc::SIGINT)));
t!(cvt_nz(libc::pthread_sigmask(
libc::SIG_SETMASK,
set.as_ptr(),
old_set.as_mut_ptr()
)));
let mut set = mem::MaybeUninit::<libc::sigset_t>::uninit();
let mut old_set = mem::MaybeUninit::<libc::sigset_t>::uninit();
t!(cvt(sigemptyset(set.as_mut_ptr())));
t!(cvt(sigaddset(set.as_mut_ptr(), libc::SIGINT)));
t!(cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, set.as_ptr(), old_set.as_mut_ptr())));
cmd.stdin(Stdio::MakePipe);
cmd.stdout(Stdio::MakePipe);
cmd.stdin(Stdio::MakePipe);
cmd.stdout(Stdio::MakePipe);
let (mut cat, mut pipes) = t!(cmd.spawn(Stdio::Null, true));
let stdin_write = pipes.stdin.take().unwrap();
let stdout_read = pipes.stdout.take().unwrap();
let (mut cat, mut pipes) = t!(cmd.spawn(Stdio::Null, true));
let stdin_write = pipes.stdin.take().unwrap();
let stdout_read = pipes.stdout.take().unwrap();
t!(cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, old_set.as_ptr(), ptr::null_mut())));
t!(cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, old_set.as_ptr(), ptr::null_mut())));
t!(cvt(libc::kill(cat.id() as libc::pid_t, libc::SIGINT)));
// We need to wait until SIGINT is definitely delivered. The
// easiest way is to write something to cat, and try to read it
// back: if SIGINT is unmasked, it'll get delivered when cat is
// next scheduled.
let _ = stdin_write.write(b"Hello");
drop(stdin_write);
t!(cvt(libc::kill(cat.id() as libc::pid_t, libc::SIGINT)));
// We need to wait until SIGINT is definitely delivered. The
// easiest way is to write something to cat, and try to read it
// back: if SIGINT is unmasked, it'll get delivered when cat is
// next scheduled.
let _ = stdin_write.write(b"Hello");
drop(stdin_write);
// Exactly 5 bytes should be read.
let mut buf = [0; 5];
let ret = t!(stdout_read.read(&mut buf));
assert_eq!(ret, 5);
assert_eq!(&buf, b"Hello");
// Either EOF or failure (EPIPE) is okay.
let mut buf = [0; 5];
if let Ok(ret) = stdout_read.read(&mut buf) {
assert_eq!(ret, 0);
t!(cat.wait());
}
t!(cat.wait());
}
// A plain `Command::new` uses the posix_spawn path on many platforms.
let cmd = Command::new(OsStr::new("cat"));
test_inner(cmd);
// Specifying `pre_exec` forces the fork/exec path.
let mut cmd = Command::new(OsStr::new("cat"));
unsafe { cmd.pre_exec(Box::new(|| Ok(()))) };
test_inner(cmd);
}
#[test]

View File

@ -2,7 +2,6 @@ use crate::fmt;
use crate::io::{self, Error, ErrorKind};
use crate::mem;
use crate::num::NonZeroI32;
use crate::ptr;
use crate::sys;
use crate::sys::cvt;
use crate::sys::process::process_common::*;
@ -310,7 +309,7 @@ impl Command {
//FIXME: Redox kernel does not support setgroups yet
#[cfg(not(target_os = "redox"))]
if libc::getuid() == 0 && self.get_groups().is_none() {
cvt(libc::setgroups(0, ptr::null()))?;
cvt(libc::setgroups(0, crate::ptr::null()))?;
}
cvt(libc::setuid(u as uid_t))?;
}
@ -326,30 +325,26 @@ 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()))?;
// Inherit the signal mask from the parent rather than resetting it (i.e. do not call
// pthread_sigmask).
#[cfg(target_os = "android")] // see issue #88585
{
let mut action: libc::sigaction = mem::zeroed();
action.sa_sigaction = libc::SIG_DFL;
cvt(libc::sigaction(libc::SIGPIPE, &action, ptr::null_mut()))?;
}
#[cfg(not(target_os = "android"))]
{
let ret = sys::signal(libc::SIGPIPE, libc::SIG_DFL);
if ret == libc::SIG_ERR {
return Err(io::Error::last_os_error());
// If #[unix_sigpipe] is specified, don't reset SIGPIPE to SIG_DFL.
// If #[unix_sigpipe] is not specified, reset SIGPIPE to SIG_DFL for backward compatibility.
//
// #[unix_sigpipe] is an opportunity to change the default here.
if !crate::sys::unix_sigpipe_attr_specified() {
#[cfg(target_os = "android")] // see issue #88585
{
let mut action: libc::sigaction = mem::zeroed();
action.sa_sigaction = libc::SIG_DFL;
cvt(libc::sigaction(libc::SIGPIPE, &action, crate::ptr::null_mut()))?;
}
#[cfg(not(target_os = "android"))]
{
let ret = sys::signal(libc::SIGPIPE, libc::SIG_DFL);
if ret == libc::SIG_ERR {
return Err(io::Error::last_os_error());
}
}
}
}
@ -411,7 +406,7 @@ impl Command {
envp: Option<&CStringArray>,
) -> io::Result<Option<Process>> {
use crate::mem::MaybeUninit;
use crate::sys::{self, cvt_nz};
use crate::sys::{self, cvt_nz, unix_sigpipe_attr_specified};
if self.get_gid().is_some()
|| self.get_uid().is_some()
@ -531,13 +526,24 @@ 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()))?;
// Inherit the signal mask from this process rather than resetting it (i.e. do not call
// posix_spawnattr_setsigmask).
// If #[unix_sigpipe] is specified, don't reset SIGPIPE to SIG_DFL.
// If #[unix_sigpipe] is not specified, reset SIGPIPE to SIG_DFL for backward compatibility.
//
// #[unix_sigpipe] is an opportunity to change the default here.
if !unix_sigpipe_attr_specified() {
let mut default_set = MaybeUninit::<libc::sigset_t>::uninit();
cvt(sigemptyset(default_set.as_mut_ptr()))?;
cvt(sigaddset(default_set.as_mut_ptr(), libc::SIGPIPE))?;
cvt_nz(libc::posix_spawnattr_setsigdefault(
attrs.0.as_mut_ptr(),
default_set.as_ptr(),
))?;
flags |= libc::POSIX_SPAWN_SETSIGDEF;
}
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

View File

@ -36,7 +36,7 @@ hello world
Set the `SIGPIPE` handler to `SIG_IGN` before invoking `fn main()`. This will result in `ErrorKind::BrokenPipe` errors if you program tries to write to a closed pipe. This is normally what you want if you for example write socket servers, socket clients, or pipe peers.
This is what libstd has done by default since 2014. Omitting `#[unix_sigpipe = "..."]` is the same as having `#[unix_sigpipe = "sig_ign"]`.
This is what libstd has done by default since 2014. (However, see the note on child processes below.)
### Example
@ -52,3 +52,11 @@ hello world
thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', library/std/src/io/stdio.rs:1016:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
### Note on child processes
When spawning child processes, the legacy Rust behavior if `#[unix_sigpipe]` is not specified is to
reset `SIGPIPE` to `SIG_DFL`.
If `#[unix_sigpipe = "..."]` is specified, no matter what its value is, the signal disposition of
`SIGPIPE` is no longer reset. This means that the child inherits the parent's `SIGPIPE` behavior.

View File

@ -23,9 +23,11 @@ pub fn assert_sigpipe_handler(expected_handler: SignalHandler) {
SignalHandler::Ignore => libc::SIG_IGN,
SignalHandler::Default => libc::SIG_DFL,
};
assert_eq!(prev, expected);
assert_eq!(prev, expected, "expected sigpipe value matches actual value");
// Unlikely to matter, but restore the old value anyway
unsafe { libc::signal(libc::SIGPIPE, prev); };
unsafe {
libc::signal(libc::SIGPIPE, prev);
};
}
}