Skip to content

Commit 021551b

Browse files
authored
Rollup merge of rust-lang#101077 - sunshowers:signal-mask-inherit, r=thomcc
Change process spawning to inherit the parent's signal mask by default Previously, the signal mask was always reset when a child process is started. This breaks tools like `nohup` which expect `SIGHUP` to be blocked for all transitive processes. 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.
2 parents d7dd01f + 49166fb commit 021551b

File tree

7 files changed

+141
-78
lines changed

7 files changed

+141
-78
lines changed

compiler/rustc_session/src/config/sigpipe.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
//! NOTE: Keep these constants in sync with `library/std/src/sys/unix/mod.rs`!
22
3+
/// The default value if `#[unix_sigpipe]` is not specified. This resolves
4+
/// to `SIG_IGN` in `library/std/src/sys/unix/mod.rs`.
5+
///
6+
/// Note that `SIG_IGN` has been the Rust default since 2014. See
7+
/// <https://github.com/rust-lang/rust/issues/62569>.
8+
#[allow(dead_code)]
9+
pub const DEFAULT: u8 = 0;
10+
311
/// Do not touch `SIGPIPE`. Use whatever the parent process uses.
412
#[allow(dead_code)]
513
pub const INHERIT: u8 = 1;
@@ -15,8 +23,3 @@ pub const SIG_IGN: u8 = 2;
1523
/// such as `head -n 1`.
1624
#[allow(dead_code)]
1725
pub const SIG_DFL: u8 = 3;
18-
19-
/// `SIG_IGN` has been the Rust default since 2014. See
20-
/// <https://github.com/rust-lang/rust/issues/62569>.
21-
#[allow(dead_code)]
22-
pub const DEFAULT: u8 = SIG_IGN;

library/std/src/rt.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ macro_rules! rtunwrap {
8989
// `src/tools/tidy/src/pal.rs` for more info. On all other platforms, `sigpipe`
9090
// has a value, but its value is ignored.
9191
//
92-
// Even though it is an `u8`, it only ever has 3 values. These are documented in
92+
// Even though it is an `u8`, it only ever has 4 values. These are documented in
9393
// `compiler/rustc_session/src/config/sigpipe.rs`.
9494
#[cfg_attr(test, allow(dead_code))]
9595
unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {

library/std/src/sys/unix/mod.rs

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
use crate::ffi::CStr;
44
use crate::io::ErrorKind;
5+
use crate::sync::atomic::{AtomicBool, Ordering};
56

67
pub use self::rand::hashmap_random_keys;
78

@@ -163,24 +164,53 @@ pub unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {
163164
// See the other file for docs. NOTE: Make sure to keep them in
164165
// sync!
165166
mod sigpipe {
167+
pub const DEFAULT: u8 = 0;
166168
pub const INHERIT: u8 = 1;
167169
pub const SIG_IGN: u8 = 2;
168170
pub const SIG_DFL: u8 = 3;
169171
}
170172

171-
let handler = match sigpipe {
172-
sigpipe::INHERIT => None,
173-
sigpipe::SIG_IGN => Some(libc::SIG_IGN),
174-
sigpipe::SIG_DFL => Some(libc::SIG_DFL),
173+
let (sigpipe_attr_specified, handler) = match sigpipe {
174+
sigpipe::DEFAULT => (false, Some(libc::SIG_IGN)),
175+
sigpipe::INHERIT => (true, None),
176+
sigpipe::SIG_IGN => (true, Some(libc::SIG_IGN)),
177+
sigpipe::SIG_DFL => (true, Some(libc::SIG_DFL)),
175178
_ => unreachable!(),
176179
};
180+
// The bootstrap compiler doesn't know about sigpipe::DEFAULT, and always passes in
181+
// SIG_IGN. This causes some tests to fail because they expect SIGPIPE to be reset to
182+
// default on process spawning (which doesn't happen if #[unix_sigpipe] is specified).
183+
// Since we can't differentiate between the cases here, treat SIG_IGN as DEFAULT
184+
// unconditionally.
185+
if sigpipe_attr_specified && !(cfg!(bootstrap) && sigpipe == sigpipe::SIG_IGN) {
186+
UNIX_SIGPIPE_ATTR_SPECIFIED.store(true, Ordering::Relaxed);
187+
}
177188
if let Some(handler) = handler {
178189
rtassert!(signal(libc::SIGPIPE, handler) != libc::SIG_ERR);
179190
}
180191
}
181192
}
182193
}
183194

195+
// This is set (up to once) in reset_sigpipe.
196+
#[cfg(not(any(
197+
target_os = "espidf",
198+
target_os = "emscripten",
199+
target_os = "fuchsia",
200+
target_os = "horizon"
201+
)))]
202+
static UNIX_SIGPIPE_ATTR_SPECIFIED: AtomicBool = AtomicBool::new(false);
203+
204+
#[cfg(not(any(
205+
target_os = "espidf",
206+
target_os = "emscripten",
207+
target_os = "fuchsia",
208+
target_os = "horizon"
209+
)))]
210+
pub(crate) fn unix_sigpipe_attr_specified() -> bool {
211+
UNIX_SIGPIPE_ATTR_SPECIFIED.load(Ordering::Relaxed)
212+
}
213+
184214
// SAFETY: must be called only once during runtime cleanup.
185215
// NOTE: this is not guaranteed to run, for example when the program aborts.
186216
pub unsafe fn cleanup() {

library/std/src/sys/unix/process/process_common/tests.rs

Lines changed: 46 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -31,41 +31,54 @@ macro_rules! t {
3131
ignore
3232
)]
3333
fn test_process_mask() {
34-
unsafe {
35-
// Test to make sure that a signal mask does not get inherited.
36-
let mut cmd = Command::new(OsStr::new("cat"));
37-
38-
let mut set = mem::MaybeUninit::<libc::sigset_t>::uninit();
39-
let mut old_set = mem::MaybeUninit::<libc::sigset_t>::uninit();
40-
t!(cvt(sigemptyset(set.as_mut_ptr())));
41-
t!(cvt(sigaddset(set.as_mut_ptr(), libc::SIGINT)));
42-
t!(cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, set.as_ptr(), old_set.as_mut_ptr())));
43-
44-
cmd.stdin(Stdio::MakePipe);
45-
cmd.stdout(Stdio::MakePipe);
46-
47-
let (mut cat, mut pipes) = t!(cmd.spawn(Stdio::Null, true));
48-
let stdin_write = pipes.stdin.take().unwrap();
49-
let stdout_read = pipes.stdout.take().unwrap();
50-
51-
t!(cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, old_set.as_ptr(), ptr::null_mut())));
52-
53-
t!(cvt(libc::kill(cat.id() as libc::pid_t, libc::SIGINT)));
54-
// We need to wait until SIGINT is definitely delivered. The
55-
// easiest way is to write something to cat, and try to read it
56-
// back: if SIGINT is unmasked, it'll get delivered when cat is
57-
// next scheduled.
58-
let _ = stdin_write.write(b"Hello");
59-
drop(stdin_write);
60-
61-
// Either EOF or failure (EPIPE) is okay.
62-
let mut buf = [0; 5];
63-
if let Ok(ret) = stdout_read.read(&mut buf) {
64-
assert_eq!(ret, 0);
34+
// Test to make sure that a signal mask *does* get inherited.
35+
fn test_inner(mut cmd: Command) {
36+
unsafe {
37+
let mut set = mem::MaybeUninit::<libc::sigset_t>::uninit();
38+
let mut old_set = mem::MaybeUninit::<libc::sigset_t>::uninit();
39+
t!(cvt(sigemptyset(set.as_mut_ptr())));
40+
t!(cvt(sigaddset(set.as_mut_ptr(), libc::SIGINT)));
41+
t!(cvt_nz(libc::pthread_sigmask(
42+
libc::SIG_SETMASK,
43+
set.as_ptr(),
44+
old_set.as_mut_ptr()
45+
)));
46+
47+
cmd.stdin(Stdio::MakePipe);
48+
cmd.stdout(Stdio::MakePipe);
49+
50+
let (mut cat, mut pipes) = t!(cmd.spawn(Stdio::Null, true));
51+
let stdin_write = pipes.stdin.take().unwrap();
52+
let stdout_read = pipes.stdout.take().unwrap();
53+
54+
t!(cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, old_set.as_ptr(), ptr::null_mut())));
55+
56+
t!(cvt(libc::kill(cat.id() as libc::pid_t, libc::SIGINT)));
57+
// We need to wait until SIGINT is definitely delivered. The
58+
// easiest way is to write something to cat, and try to read it
59+
// back: if SIGINT is unmasked, it'll get delivered when cat is
60+
// next scheduled.
61+
let _ = stdin_write.write(b"Hello");
62+
drop(stdin_write);
63+
64+
// Exactly 5 bytes should be read.
65+
let mut buf = [0; 5];
66+
let ret = t!(stdout_read.read(&mut buf));
67+
assert_eq!(ret, 5);
68+
assert_eq!(&buf, b"Hello");
69+
70+
t!(cat.wait());
6571
}
66-
67-
t!(cat.wait());
6872
}
73+
74+
// A plain `Command::new` uses the posix_spawn path on many platforms.
75+
let cmd = Command::new(OsStr::new("cat"));
76+
test_inner(cmd);
77+
78+
// Specifying `pre_exec` forces the fork/exec path.
79+
let mut cmd = Command::new(OsStr::new("cat"));
80+
unsafe { cmd.pre_exec(Box::new(|| Ok(()))) };
81+
test_inner(cmd);
6982
}
7083

7184
#[test]

library/std/src/sys/unix/process/process_unix.rs

Lines changed: 39 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ impl Command {
278278
stdio: ChildPipes,
279279
maybe_envp: Option<&CStringArray>,
280280
) -> Result<!, io::Error> {
281-
use crate::sys::{self, cvt_r};
281+
use crate::sys::{self, cvt_r, unix_sigpipe_attr_specified};
282282

283283
if let Some(fd) = stdio.stdin.fd() {
284284
cvt_r(|| libc::dup2(fd, libc::STDIN_FILENO))?;
@@ -326,30 +326,26 @@ impl Command {
326326
// emscripten has no signal support.
327327
#[cfg(not(target_os = "emscripten"))]
328328
{
329-
use crate::mem::MaybeUninit;
330-
use crate::sys::cvt_nz;
331-
// Reset signal handling so the child process starts in a
332-
// standardized state. libstd ignores SIGPIPE, and signal-handling
333-
// libraries often set a mask. Child processes inherit ignored
334-
// signals and the signal mask from their parent, but most
335-
// UNIX programs do not reset these things on their own, so we
336-
// need to clean things up now to avoid confusing the program
337-
// we're about to run.
338-
let mut set = MaybeUninit::<libc::sigset_t>::uninit();
339-
cvt(sigemptyset(set.as_mut_ptr()))?;
340-
cvt_nz(libc::pthread_sigmask(libc::SIG_SETMASK, set.as_ptr(), ptr::null_mut()))?;
341-
342-
#[cfg(target_os = "android")] // see issue #88585
343-
{
344-
let mut action: libc::sigaction = mem::zeroed();
345-
action.sa_sigaction = libc::SIG_DFL;
346-
cvt(libc::sigaction(libc::SIGPIPE, &action, ptr::null_mut()))?;
347-
}
348-
#[cfg(not(target_os = "android"))]
349-
{
350-
let ret = sys::signal(libc::SIGPIPE, libc::SIG_DFL);
351-
if ret == libc::SIG_ERR {
352-
return Err(io::Error::last_os_error());
329+
// Inherit the signal mask from the parent rather than resetting it (i.e. do not call
330+
// pthread_sigmask).
331+
332+
// If #[unix_sigpipe] is specified, don't reset SIGPIPE to SIG_DFL.
333+
// If #[unix_sigpipe] is not specified, reset SIGPIPE to SIG_DFL for backward compatibility.
334+
//
335+
// #[unix_sigpipe] is an opportunity to change the default here.
336+
if !unix_sigpipe_attr_specified() {
337+
#[cfg(target_os = "android")] // see issue #88585
338+
{
339+
let mut action: libc::sigaction = mem::zeroed();
340+
action.sa_sigaction = libc::SIG_DFL;
341+
cvt(libc::sigaction(libc::SIGPIPE, &action, ptr::null_mut()))?;
342+
}
343+
#[cfg(not(target_os = "android"))]
344+
{
345+
let ret = sys::signal(libc::SIGPIPE, libc::SIG_DFL);
346+
if ret == libc::SIG_ERR {
347+
return Err(io::Error::last_os_error());
348+
}
353349
}
354350
}
355351
}
@@ -411,7 +407,7 @@ impl Command {
411407
envp: Option<&CStringArray>,
412408
) -> io::Result<Option<Process>> {
413409
use crate::mem::MaybeUninit;
414-
use crate::sys::{self, cvt_nz};
410+
use crate::sys::{self, cvt_nz, unix_sigpipe_attr_specified};
415411

416412
if self.get_gid().is_some()
417413
|| self.get_uid().is_some()
@@ -531,13 +527,24 @@ impl Command {
531527
cvt_nz(libc::posix_spawnattr_setpgroup(attrs.0.as_mut_ptr(), pgroup))?;
532528
}
533529

534-
let mut set = MaybeUninit::<libc::sigset_t>::uninit();
535-
cvt(sigemptyset(set.as_mut_ptr()))?;
536-
cvt_nz(libc::posix_spawnattr_setsigmask(attrs.0.as_mut_ptr(), set.as_ptr()))?;
537-
cvt(sigaddset(set.as_mut_ptr(), libc::SIGPIPE))?;
538-
cvt_nz(libc::posix_spawnattr_setsigdefault(attrs.0.as_mut_ptr(), set.as_ptr()))?;
530+
// Inherit the signal mask from this process rather than resetting it (i.e. do not call
531+
// posix_spawnattr_setsigmask).
532+
533+
// If #[unix_sigpipe] is specified, don't reset SIGPIPE to SIG_DFL.
534+
// If #[unix_sigpipe] is not specified, reset SIGPIPE to SIG_DFL for backward compatibility.
535+
//
536+
// #[unix_sigpipe] is an opportunity to change the default here.
537+
if !unix_sigpipe_attr_specified() {
538+
let mut default_set = MaybeUninit::<libc::sigset_t>::uninit();
539+
cvt(sigemptyset(default_set.as_mut_ptr()))?;
540+
cvt(sigaddset(default_set.as_mut_ptr(), libc::SIGPIPE))?;
541+
cvt_nz(libc::posix_spawnattr_setsigdefault(
542+
attrs.0.as_mut_ptr(),
543+
default_set.as_ptr(),
544+
))?;
545+
flags |= libc::POSIX_SPAWN_SETSIGDEF;
546+
}
539547

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

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

src/doc/unstable-book/src/language-features/unix-sigpipe.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ hello world
3636

3737
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.
3838

39-
This is what libstd has done by default since 2014. Omitting `#[unix_sigpipe = "..."]` is the same as having `#[unix_sigpipe = "sig_ign"]`.
39+
This is what libstd has done by default since 2014. (However, see the note on child processes below.)
4040

4141
### Example
4242

@@ -52,3 +52,11 @@ hello world
5252
thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', library/std/src/io/stdio.rs:1016:9
5353
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
5454
```
55+
56+
### Note on child processes
57+
58+
When spawning child processes, the legacy Rust behavior if `#[unix_sigpipe]` is not specified is to
59+
reset `SIGPIPE` to `SIG_DFL`.
60+
61+
If `#[unix_sigpipe = "..."]` is specified, no matter what its value is, the signal disposition of
62+
`SIGPIPE` is no longer reset. This means that the child inherits the parent's `SIGPIPE` behavior.

src/test/ui/attributes/unix_sigpipe/auxiliary/sigpipe-utils.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@ pub fn assert_sigpipe_handler(expected_handler: SignalHandler) {
2323
SignalHandler::Ignore => libc::SIG_IGN,
2424
SignalHandler::Default => libc::SIG_DFL,
2525
};
26-
assert_eq!(prev, expected);
26+
assert_eq!(prev, expected, "expected sigpipe value matches actual value");
2727

2828
// Unlikely to matter, but restore the old value anyway
29-
unsafe { libc::signal(libc::SIGPIPE, prev); };
29+
unsafe {
30+
libc::signal(libc::SIGPIPE, prev);
31+
};
3032
}
3133
}

0 commit comments

Comments
 (0)