diff --git a/src/exec.rs b/src/exec.rs index 3a41f0d1e..9df222fd7 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -40,7 +40,7 @@ print_exit_warning_for_jobs, InternalProc, Job, JobGroupRef, 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(); + safe_restore_term_mode(); // Bounce to launch_process. This never returns. safe_launch_process(p, &actual_cmd, &argv, &envp); } diff --git a/src/key.rs b/src/key.rs index d7dbeefd6..8f85bea8d 100644 --- a/src/key.rs +++ b/src/key.rs @@ -5,7 +5,7 @@ fallback::fish_wcwidth, flog::FloggableDebug, future_feature_flags::{test as feature_test, FeatureFlag}, - reader::TERMINAL_MODE_ON_STARTUP, + reader::safe_get_terminal_mode_on_startup, wchar::{decode_byte_from_char, prelude::*}, wutil::{fish_is_pua, fish_wcstoul}, }; @@ -179,8 +179,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 diff --git a/src/reader.rs b/src/reader.rs index 7428cd9b8..dfaeb48a1 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -39,8 +39,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}; @@ -177,9 +176,9 @@ enum ExitState { pub static SHELL_MODES: Lazy> = Lazy::new(|| Mutex::new(unsafe { std::mem::zeroed() })); -/// Mode on startup, which we restore on exit. -pub static TERMINAL_MODE_ON_STARTUP: Lazy> = - 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 = AtomicPtr::new(std::ptr::null_mut()); /// Mode we use to execute programs. static TTY_MODES_FOR_EXTERNAL_CMDS: Lazy> = @@ -196,6 +195,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> { @@ -857,8 +862,15 @@ fn read_ni(parser: &Parser, fd: RawFd, io: &IoChain) -> Result<(), ErrorCode> { /// 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::() }; + 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()); @@ -866,7 +878,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. @@ -878,7 +890,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, @@ -893,7 +904,7 @@ 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(restore_foreground_pgroup: bool) { - restore_term_mode(); + safe_restore_term_mode(); crate::input_common::terminal_protocols_disable_ifn(); if restore_foreground_pgroup { restore_term_foreground_process_group_for_exit(); @@ -903,18 +914,14 @@ 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() { +/// THIS FUNCTION IS CALLED FROM A SIGNAL HANDLER. IT MUST BE ASYNC-SIGNAL-SAFE. +pub fn safe_restore_term_mode() { if !is_interactive_session() || getpgrp() != unsafe { libc::tcgetpgrp(STDIN_FILENO) } { return; } - - unsafe { - libc::tcsetattr( - STDIN_FILENO, - TCSANOW, - &*TERMINAL_MODE_ON_STARTUP.lock().unwrap(), - ) - }; + if let Some(modes) = safe_get_terminal_mode_on_startup() { + unsafe { libc::tcsetattr(STDIN_FILENO, TCSANOW, modes) }; + } } /// Change the history file for the current command reading context. diff --git a/src/signal.rs b/src/signal.rs index bce91db58..45b7f4346 100644 --- a/src/signal.rs +++ b/src/signal.rs @@ -4,8 +4,7 @@ use crate::common::exit_without_destructors; use crate::event::{enqueue_signal, is_signal_observed}; use crate::nix::getpid; -use crate::panic::AT_EXIT; -use crate::reader::{reader_handle_sigint, reader_sighup}; +use crate::reader::{reader_handle_sigint, reader_sighup, safe_restore_term_mode}; use crate::termsize::TermsizeContainer; use crate::topic_monitor::{topic_monitor_principal, Generation, GenerationsList, Topic}; use crate::wchar::prelude::*; @@ -90,9 +89,7 @@ extern "C" fn fish_signal_handler( libc::SIGTERM => { // 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)(); - } + safe_restore_term_mode(); // Safety: signal() and raise() are async-signal-safe. unsafe { libc::signal(libc::SIGTERM, libc::SIG_DFL);