From 30cba03bf94452e7a6f38c776f113f62d9ee93f4 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Mon, 21 Oct 2024 09:16:02 +0200 Subject: [PATCH] Make SIGTERM handler async-signal-safe again --- src/common.rs | 12 ++++++++---- src/exec.rs | 2 +- src/fork_exec/mod.rs | 2 +- src/input_common.rs | 16 ++++------------ src/panic.rs | 6 +++--- src/proc.rs | 2 +- src/reader.rs | 34 +++++++++++++++++++--------------- src/signal.rs | 2 +- src/tests/mod.rs | 2 +- 9 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/common.rs b/src/common.rs index 8e6906648..025ab99b2 100644 --- a/src/common.rs +++ b/src/common.rs @@ -21,7 +21,6 @@ use bitflags::bitflags; use core::slice; use libc::{EIO, O_WRONLY, SIGTTOU, SIG_IGN, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use once_cell::sync::OnceCell; -use std::env; use std::ffi::{CStr, CString, OsStr, OsString}; use std::mem; use std::ops::{Deref, DerefMut}; @@ -30,6 +29,7 @@ use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicI32, AtomicU32, Ordering}; use std::sync::{Arc, MutexGuard}; use std::time; +use std::{env, process}; pub const PACKAGE_NAME: &str = env!("CARGO_PKG_NAME"); @@ -1634,12 +1634,16 @@ pub fn fish_reserved_codepoint(c: char) -> bool { || (c >= key::Backspace && c < ENCODE_DIRECT_END) } -pub fn redirect_tty_output() { +pub fn redirect_tty_output(in_signal_handler: bool) { unsafe { let mut t: libc::termios = mem::zeroed(); - let s = CString::new("/dev/null").unwrap(); + let s = CStr::from_bytes_with_nul(b"/dev/null\0").unwrap(); let fd = libc::open(s.as_ptr(), O_WRONLY); - assert!(fd != -1, "Could not open /dev/null!"); + if in_signal_handler && fd == -1 { + process::abort(); + } else { + assert!(fd != -1, "Could not open /dev/null!"); + } for stdfd in [STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO] { if libc::tcgetattr(stdfd, &mut t) == -1 && errno::errno().0 == EIO { libc::dup2(fd, stdfd); diff --git a/src/exec.rs b/src/exec.rs index 7ccdf94e0..46625d0a9 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -440,7 +440,7 @@ fn launch_process_nofork(vars: &EnvStack, p: &Process) -> ! { let actual_cmd = wcs2zstring(&p.actual_cmd); // Ensure the terminal modes are what they were before we changed them. - restore_term_mode(); + restore_term_mode(false); // Bounce to launch_process. This never returns. safe_launch_process(p, &actual_cmd, &argv, &*envp); } diff --git a/src/fork_exec/mod.rs b/src/fork_exec/mod.rs index 55b818570..2c10fccd5 100644 --- a/src/fork_exec/mod.rs +++ b/src/fork_exec/mod.rs @@ -2,7 +2,7 @@ // This concerns posix_spawn support, and async-signal // safe code which happens in between fork and exec. -mod flog_safe; +pub mod flog_safe; pub mod postfork; pub mod spawn; use crate::proc::Job; diff --git a/src/input_common.rs b/src/input_common.rs index 86c9a1d1e..c67d00ccf 100644 --- a/src/input_common.rs +++ b/src/input_common.rs @@ -7,6 +7,7 @@ use crate::common::{ use crate::env::{EnvStack, Environment}; use crate::fd_readable_set::FdReadableSet; use crate::flog::FLOG; +use crate::fork_exec::flog_safe::FLOG_SAFE; use crate::global_safety::RelaxedAtomicBool; use crate::key::{ self, alt, canonicalize_control_char, canonicalize_keyed_control_char, function_key, shift, @@ -485,13 +486,7 @@ pub fn terminal_protocols_enable_ifn() { "\x1b=", // set application keypad mode, so the keypad keys send unique codes ) }; - FLOG!( - term_protocols, - format!( - "Enabling extended keys and bracketed paste: {:?}", - sequences - ) - ); + FLOG!(term_protocols, "Enabling extended keys and bracketed paste"); let _ = write_to_fd(sequences.as_bytes(), STDOUT_FILENO); if IS_TMUX.load() { let _ = write_to_fd("\x1b[?1004h".as_bytes(), STDOUT_FILENO); @@ -513,12 +508,9 @@ pub(crate) fn terminal_protocols_disable_ifn() { "\x1b>", // application keypad mode ) }; - FLOG!( + FLOG_SAFE!( term_protocols, - format!( - "Disabling extended keys and bracketed paste: {:?}", - sequences - ) + "Disabling extended keys and bracketed paste" ); let _ = write_to_fd(sequences.as_bytes(), STDOUT_FILENO); if IS_TMUX.load() { diff --git a/src/panic.rs b/src/panic.rs index 4ee4bf4dd..04e7db38d 100644 --- a/src/panic.rs +++ b/src/panic.rs @@ -13,7 +13,7 @@ use crate::{ threads::{asan_maybe_exit, is_main_thread}, }; -pub static AT_EXIT: OnceCell> = OnceCell::new(); +pub static AT_EXIT: OnceCell> = OnceCell::new(); pub fn panic_handler(main: impl FnOnce() -> i32 + UnwindSafe) -> ! { if isatty(STDIN_FILENO) { @@ -28,7 +28,7 @@ pub fn panic_handler(main: impl FnOnce() -> i32 + UnwindSafe) -> ! { return; } if let Some(at_exit) = AT_EXIT.get() { - (at_exit)(); + (at_exit)(false); } eprintf!( "%s crashed, please report a bug.", @@ -54,7 +54,7 @@ pub fn panic_handler(main: impl FnOnce() -> i32 + UnwindSafe) -> ! { } let exit_status = main(); if let Some(at_exit) = AT_EXIT.get() { - (at_exit)(); + (at_exit)(false); } asan_maybe_exit(exit_status); std::process::exit(exit_status) diff --git a/src/proc.rs b/src/proc.rs index 3c11b6382..e154beec3 100644 --- a/src/proc.rs +++ b/src/proc.rs @@ -425,7 +425,7 @@ impl TtyTransfer { } EBADF => { // stdin has been closed. Workaround a glibc bug - see #3644. - redirect_tty_output(); + redirect_tty_output(false); return false; } _ => { diff --git a/src/reader.rs b/src/reader.rs index a817d0e63..7cb30af45 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -200,7 +200,7 @@ fn redirect_tty_after_sighup() { assert!(reader_received_sighup(), "SIGHUP not received"); static TTY_REDIRECTED: RelaxedAtomicBool = RelaxedAtomicBool::new(false); if !TTY_REDIRECTED.swap(true) { - redirect_tty_output(); + redirect_tty_output(false); } } @@ -585,7 +585,7 @@ pub fn reader_read(parser: &Parser, fd: RawFd, io: &IoChain) -> c_int { if isatty(STDIN_FILENO) { interactive = true; } else if unsafe { libc::tcgetattr(STDIN_FILENO, &mut t) } == -1 && errno().0 == EIO { - redirect_tty_output(); + redirect_tty_output(false); interactive = true; } } @@ -793,7 +793,11 @@ pub fn reader_init(will_restore_foreground_pgroup: bool) { #[cfg(not(test))] assert!(AT_EXIT.get().is_none()); - AT_EXIT.get_or_init(|| Box::new(move || reader_deinit(will_restore_foreground_pgroup))); + AT_EXIT.get_or_init(|| { + Box::new(move |in_signal_handler| { + reader_deinit(in_signal_handler, will_restore_foreground_pgroup) + }) + }); // Set the mode used for program execution, initialized to the current mode. let mut tty_modes_for_external_cmds = TTY_MODES_FOR_EXTERNAL_CMDS.lock().unwrap(); @@ -821,8 +825,8 @@ pub fn reader_init(will_restore_foreground_pgroup: bool) { } } -pub fn reader_deinit(restore_foreground_pgroup: bool) { - restore_term_mode(); +pub fn reader_deinit(in_signal_handler: bool, restore_foreground_pgroup: bool) { + restore_term_mode(in_signal_handler); crate::input_common::terminal_protocols_disable_ifn(); if restore_foreground_pgroup { restore_term_foreground_process_group_for_exit(); @@ -832,7 +836,7 @@ pub fn reader_deinit(restore_foreground_pgroup: bool) { /// Restore the term mode if we own the terminal and are interactive (#8705). /// It's important we do this before restore_foreground_process_group, /// otherwise we won't think we own the terminal. -pub fn restore_term_mode() { +pub fn restore_term_mode(in_signal_handler: bool) { if !is_interactive_session() || unsafe { libc::getpgrp() != libc::tcgetpgrp(STDIN_FILENO) } { return; } @@ -845,7 +849,7 @@ pub fn restore_term_mode() { ) == -1 } && errno().0 == EIO { - redirect_tty_output(); + redirect_tty_output(in_signal_handler); } } @@ -1889,14 +1893,14 @@ impl<'a> Reader<'a> { // Get the current terminal modes. These will be restored when the function returns. let mut old_modes: libc::termios = unsafe { std::mem::zeroed() }; if unsafe { libc::tcgetattr(zelf.conf.inputfd, &mut old_modes) } == -1 && errno().0 == EIO { - redirect_tty_output(); + redirect_tty_output(false); } // Set the new modes. if unsafe { libc::tcsetattr(zelf.conf.inputfd, TCSANOW, &*shell_modes()) } == -1 { let err = errno().0; if err == EIO { - redirect_tty_output(); + redirect_tty_output(false); } // This check is required to work around certain issues with fish's approach to @@ -1974,7 +1978,7 @@ impl<'a> Reader<'a> { && is_interactive_session() { if errno().0 == EIO { - redirect_tty_output(); + redirect_tty_output(false); } perror("tcsetattr"); // return to previous mode } @@ -3758,7 +3762,7 @@ fn term_donate(quiet: bool /* = false */) { } == -1 { if errno().0 == EIO { - redirect_tty_output(); + redirect_tty_output(false); } if errno().0 != EINTR { if !quiet { @@ -3804,7 +3808,7 @@ fn term_steal(copy_modes: bool) { } while unsafe { libc::tcsetattr(STDIN_FILENO, TCSANOW, &*shell_modes()) } == -1 { if errno().0 == EIO { - redirect_tty_output(); + redirect_tty_output(false); } if errno().0 != EINTR { FLOG!(warning, wgettext!("Could not set terminal mode for shell")); @@ -3875,7 +3879,7 @@ fn acquire_tty_or_exit(shell_pgid: libc::pid_t) { break; } // No TTY, cannot be interactive? - redirect_tty_output(); + redirect_tty_output(false); FLOG!( warning, wgettext!("No TTY for interactive shell (tcgetpgrp failed)") @@ -3941,7 +3945,7 @@ fn reader_interactive_init(parser: &Parser) { // Take control of the terminal if unsafe { libc::tcsetpgrp(STDIN_FILENO, shell_pgid) } == -1 { if errno().0 == ENOTTY { - redirect_tty_output(); + redirect_tty_output(false); } FLOG!(error, wgettext!("Failed to take control of the terminal")); perror("tcsetpgrp"); @@ -3951,7 +3955,7 @@ fn reader_interactive_init(parser: &Parser) { // Configure terminal attributes if unsafe { libc::tcsetattr(STDIN_FILENO, TCSANOW, &*shell_modes()) } == -1 { if errno().0 == EIO { - redirect_tty_output(); + redirect_tty_output(false); } FLOG!(warning, wgettext!("Failed to set startup terminal mode!")); perror("tcsetattr"); diff --git a/src/signal.rs b/src/signal.rs index de423ac47..0d34c8b56 100644 --- a/src/signal.rs +++ b/src/signal.rs @@ -89,7 +89,7 @@ extern "C" fn fish_signal_handler( // Handle sigterm. The only thing we do is restore the front process ID, then die. if !observed { if let Some(at_exit) = AT_EXIT.get() { - (at_exit)(); + (at_exit)(true); } // Safety: signal() and raise() are async-signal-safe. unsafe { diff --git a/src/tests/mod.rs b/src/tests/mod.rs index f2f0f73e5..e8c71775d 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -111,7 +111,7 @@ pub mod prelude { }); reader_init(false); ScopeGuard::new((), |()| { - reader_deinit(false); + reader_deinit(false, false); }) }