mirror of
https://github.com/fish-shell/fish-shell.git
synced 2026-06-06 08:51:14 -03:00
Fix async-signal safety in SIGTERM handler
Cherry-picked from -941701da3d(Restore some async-signal discipline to SIGTERM, 2025-06-15) - 81d45caa76e (Restore terminal state on SIGTERM again, 2025-06-21) Also, be more careful in terminal_protocols_disable_ifn about accessing reader_current_data(), as pointed out in65a4cb5245(Revert "Restore terminal state on SIGTERM again", 2025-07-19). See #11597
This commit is contained in:
@@ -148,7 +148,7 @@ pub fn fg(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Optio
|
||||
let job_group = job.group();
|
||||
job_group.set_is_foreground(true);
|
||||
if job.entitled_to_terminal() {
|
||||
crate::input_common::terminal_protocols_disable_ifn();
|
||||
crate::input_common::terminal_protocols_disable_ifn(false);
|
||||
}
|
||||
let tmodes = job_group.tmodes.borrow();
|
||||
if job_group.wants_terminal() && tmodes.is_some() {
|
||||
|
||||
@@ -248,7 +248,7 @@ fn read_interactive(
|
||||
|
||||
reader_readline(parser, nchars)
|
||||
};
|
||||
terminal_protocols_disable_ifn();
|
||||
terminal_protocols_disable_ifn(false);
|
||||
if let Some(line) = mline {
|
||||
*buff = line;
|
||||
if nchars > 0 && nchars < buff.len() {
|
||||
|
||||
@@ -39,7 +39,7 @@
|
||||
print_exit_warning_for_jobs, InternalProc, Job, JobGroupRef, Pid, ProcStatus, Process,
|
||||
ProcessType, TtyTransfer,
|
||||
};
|
||||
use crate::reader::{reader_run_count, restore_term_mode};
|
||||
use crate::reader::{reader_run_count, safe_restore_term_mode};
|
||||
use crate::redirection::{dup2_list_resolve_chain, Dup2List};
|
||||
use crate::threads::{iothread_perform_cant_wait, is_forked_child};
|
||||
use crate::trace::trace_if_enabled_with_args;
|
||||
@@ -437,7 +437,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(false);
|
||||
safe_restore_term_mode(false);
|
||||
// Bounce to launch_process. This never returns.
|
||||
safe_launch_process(p, &actual_cmd, &argv, &*envp);
|
||||
}
|
||||
|
||||
@@ -749,7 +749,7 @@ pub fn terminal_protocols_enable_ifn() {
|
||||
reader_current_data().map(|data| data.save_screen_state());
|
||||
}
|
||||
|
||||
pub(crate) fn terminal_protocols_disable_ifn() {
|
||||
pub(crate) fn terminal_protocols_disable_ifn(in_signal_handler: bool) {
|
||||
if !TERMINAL_PROTOCOLS.load(Ordering::Acquire) {
|
||||
return;
|
||||
}
|
||||
@@ -784,7 +784,7 @@ pub(crate) fn terminal_protocols_disable_ifn() {
|
||||
if IS_TMUX.load() {
|
||||
let _ = write_loop(&STDOUT_FILENO, "\x1b[?1004l".as_bytes());
|
||||
}
|
||||
if is_main_thread() {
|
||||
if !in_signal_handler && is_main_thread() {
|
||||
reader_current_data().map(|data| data.save_screen_state());
|
||||
}
|
||||
TERMINAL_PROTOCOLS.store(false, Ordering::Release);
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
use crate::{
|
||||
common::{escape_string, EscapeFlags, EscapeStringStyle},
|
||||
fallback::fish_wcwidth,
|
||||
reader::TERMINAL_MODE_ON_STARTUP,
|
||||
reader::safe_get_terminal_mode_on_startup,
|
||||
wchar::{decode_byte_from_char, prelude::*},
|
||||
wutil::{fish_is_pua, fish_wcstoul},
|
||||
};
|
||||
@@ -169,8 +169,10 @@ pub(crate) fn canonicalize_keyed_control_char(c: char) -> char {
|
||||
if c == ' ' {
|
||||
return Space;
|
||||
}
|
||||
if c == char::from(TERMINAL_MODE_ON_STARTUP.lock().unwrap().c_cc[VERASE]) {
|
||||
return Backspace;
|
||||
if let Some(tm) = safe_get_terminal_mode_on_startup() {
|
||||
if c == char::from(tm.c_cc[VERASE]) {
|
||||
return Backspace;
|
||||
}
|
||||
}
|
||||
if c == char::from(127) {
|
||||
// when it's not backspace
|
||||
|
||||
@@ -614,7 +614,7 @@ pub fn eval_node<T: Node>(
|
||||
let mut execution_context =
|
||||
ExecutionContext::new(ps.clone(), block_io.clone(), Rc::clone(&line_counter));
|
||||
|
||||
terminal_protocols_disable_ifn();
|
||||
terminal_protocols_disable_ifn(false);
|
||||
|
||||
// Check the exec count so we know if anything got executed.
|
||||
let prev_exec_count = self.libdata().exec_count;
|
||||
|
||||
@@ -34,8 +34,7 @@
|
||||
use std::rc::Rc;
|
||||
#[cfg(target_has_atomic = "64")]
|
||||
use std::sync::atomic::AtomicU64;
|
||||
use std::sync::atomic::Ordering;
|
||||
use std::sync::atomic::{AtomicI32, AtomicU32, AtomicU8};
|
||||
use std::sync::atomic::{AtomicI32, AtomicPtr, AtomicU32, AtomicU8, Ordering};
|
||||
use std::sync::{Arc, Mutex, MutexGuard};
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
@@ -152,9 +151,9 @@ enum ExitState {
|
||||
pub static SHELL_MODES: Lazy<Mutex<libc::termios>> =
|
||||
Lazy::new(|| Mutex::new(unsafe { std::mem::zeroed() }));
|
||||
|
||||
/// Mode on startup, which we restore on exit.
|
||||
pub static TERMINAL_MODE_ON_STARTUP: Lazy<Mutex<libc::termios>> =
|
||||
Lazy::new(|| Mutex::new(unsafe { std::mem::zeroed() }));
|
||||
/// The valid terminal modes on startup. This is set once and not modified after.
|
||||
/// Warning: this is read from the SIGTERM handler! Hence the raw global.
|
||||
static TERMINAL_MODE_ON_STARTUP: AtomicPtr<libc::termios> = AtomicPtr::new(std::ptr::null_mut());
|
||||
|
||||
/// Mode we use to execute programs.
|
||||
static TTY_MODES_FOR_EXTERNAL_CMDS: Lazy<Mutex<libc::termios>> =
|
||||
@@ -171,6 +170,12 @@ enum ExitState {
|
||||
/// This is set from a signal handler.
|
||||
static SIGHUP_RECEIVED: RelaxedAtomicBool = RelaxedAtomicBool::new(false);
|
||||
|
||||
// Get the terminal mode on startup. This is "safe" because it's async-signal safe.
|
||||
pub fn safe_get_terminal_mode_on_startup() -> Option<&'static libc::termios> {
|
||||
// Safety: set atomically and not modified after.
|
||||
unsafe { TERMINAL_MODE_ON_STARTUP.load(Ordering::Acquire).as_ref() }
|
||||
}
|
||||
|
||||
/// A singleton snapshot of the reader state. This is factored out for thread-safety reasons:
|
||||
/// it may be fetched on a background thread.
|
||||
fn commandline_state_snapshot() -> MutexGuard<'static, CommandlineState> {
|
||||
@@ -813,8 +818,15 @@ fn read_ni(parser: &Parser, fd: RawFd, io: &IoChain) -> i32 {
|
||||
/// Initialize the reader.
|
||||
pub fn reader_init(will_restore_foreground_pgroup: bool) {
|
||||
// Save the initial terminal mode.
|
||||
let mut terminal_mode_on_startup = TERMINAL_MODE_ON_STARTUP.lock().unwrap();
|
||||
unsafe { libc::tcgetattr(STDIN_FILENO, &mut *terminal_mode_on_startup) };
|
||||
// Note this field is read by a signal handler, so do it atomically, with a leaked mode.
|
||||
let mut terminal_mode_on_startup = unsafe { std::mem::zeroed::<libc::termios>() };
|
||||
let ret = unsafe { libc::tcgetattr(libc::STDIN_FILENO, &mut terminal_mode_on_startup) };
|
||||
// TODO: rationalize behavior if initial tcgetattr() fails.
|
||||
if ret == 0 {
|
||||
// Must be mut because AtomicPtr doesn't have const variant.
|
||||
let leaked: *mut libc::termios = Box::leak(Box::new(terminal_mode_on_startup));
|
||||
TERMINAL_MODE_ON_STARTUP.store(leaked, Ordering::Release);
|
||||
}
|
||||
|
||||
#[cfg(not(test))]
|
||||
assert!(AT_EXIT.get().is_none());
|
||||
@@ -826,7 +838,7 @@ pub fn reader_init(will_restore_foreground_pgroup: bool) {
|
||||
|
||||
// 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();
|
||||
*tty_modes_for_external_cmds = *terminal_mode_on_startup;
|
||||
*tty_modes_for_external_cmds = terminal_mode_on_startup;
|
||||
term_fix_external_modes(&mut tty_modes_for_external_cmds);
|
||||
|
||||
// Disable flow control by default.
|
||||
@@ -838,7 +850,6 @@ pub fn reader_init(will_restore_foreground_pgroup: bool) {
|
||||
|
||||
term_fix_modes(&mut shell_modes());
|
||||
|
||||
drop(terminal_mode_on_startup);
|
||||
drop(tty_modes_for_external_cmds);
|
||||
|
||||
// Set up our fixed terminal modes once,
|
||||
@@ -850,9 +861,11 @@ pub fn reader_init(will_restore_foreground_pgroup: bool) {
|
||||
}
|
||||
}
|
||||
|
||||
// TODO(pca): this is run in our "AT_EXIT" handler from a SIGTERM handler.
|
||||
// It must be made async-signal-safe (or not invoked).
|
||||
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();
|
||||
safe_restore_term_mode(in_signal_handler);
|
||||
crate::input_common::terminal_protocols_disable_ifn(in_signal_handler);
|
||||
if restore_foreground_pgroup {
|
||||
restore_term_foreground_process_group_for_exit();
|
||||
}
|
||||
@@ -861,20 +874,15 @@ pub fn reader_deinit(in_signal_handler: bool, 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(in_signal_handler: bool) {
|
||||
/// THIS FUNCTION IS CALLED FROM A SIGNAL HANDLER. IT MUST BE ASYNC-SIGNAL-SAFE.
|
||||
pub fn safe_restore_term_mode(in_signal_handler: bool) {
|
||||
if !is_interactive_session() || unsafe { libc::getpgrp() != libc::tcgetpgrp(STDIN_FILENO) } {
|
||||
return;
|
||||
}
|
||||
|
||||
if unsafe {
|
||||
libc::tcsetattr(
|
||||
STDIN_FILENO,
|
||||
TCSANOW,
|
||||
&*TERMINAL_MODE_ON_STARTUP.lock().unwrap(),
|
||||
) == -1
|
||||
} && errno().0 == EIO
|
||||
{
|
||||
redirect_tty_output(in_signal_handler);
|
||||
if let Some(modes) = safe_get_terminal_mode_on_startup() {
|
||||
if unsafe { libc::tcsetattr(STDIN_FILENO, TCSANOW, modes) == -1 } && errno().0 == EIO {
|
||||
redirect_tty_output(in_signal_handler);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5812,7 +5820,7 @@ fn compute_and_apply_completions(&mut self, c: ReadlineCmd) {
|
||||
token_range.end += cmdsub_range.start;
|
||||
|
||||
// Wildcard expansion and completion below check for cancellation.
|
||||
terminal_protocols_disable_ifn();
|
||||
terminal_protocols_disable_ifn(false);
|
||||
|
||||
// Check if we have a wildcard within this string; if so we first attempt to expand the
|
||||
// wildcard; if that succeeds we don't then apply user completions (#8593).
|
||||
|
||||
Reference in New Issue
Block a user