Remove bits for async-signal-safety of old SIGTERM handler

This is implied by the parent commit.

To enable this, stop trying to run cleanup in panic handlers if we
panic on a background thread.
This commit is contained in:
Johannes Altmanninger
2026-04-11 18:20:20 +08:00
parent b99ae291d6
commit 1286745e78
7 changed files with 43 additions and 49 deletions

View File

@@ -36,7 +36,7 @@
InternalProc, Job, JobGroupRef, Pid, ProcStatus, Process, ProcessType, hup_jobs,
is_interactive_session, jobs_requiring_warning_on_exit, no_exec, print_exit_warning_for_jobs,
};
use crate::reader::{reader_run_count, safe_restore_term_mode};
use crate::reader::{reader_run_count, restore_term_mode};
use crate::redirection::{Dup2List, dup2_list_resolve_chain};
use crate::threads::{ThreadPool, is_forked_child};
use crate::trace::trace_if_enabled_with_args;
@@ -449,7 +449,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.
safe_restore_term_mode();
restore_term_mode();
// Bounce to launch_process. This never returns.
safe_launch_process(p, &actual_cmd, &argv, &envp);
}

View File

@@ -1,8 +1,8 @@
use libc::VERASE;
use crate::{
flog::FloggableDebug, localizable_string, reader::safe_get_terminal_mode_on_startup,
wgettext_fmt, wutil::fish_wcstoul,
flog::FloggableDebug, localizable_string, reader::get_terminal_mode_on_startup, wgettext_fmt,
wutil::fish_wcstoul,
};
use fish_common::{EscapeFlags, EscapeStringStyle, escape_string};
use fish_fallback::fish_wcwidth;
@@ -195,7 +195,7 @@ pub(crate) fn canonicalize_keyed_control_char(c: char) -> char {
if c == ' ' {
return Space;
}
if let Some(tm) = safe_get_terminal_mode_on_startup() {
if let Some(tm) = get_terminal_mode_on_startup() {
if c == char::from(tm.c_cc[VERASE]) {
return Backspace;
}

View File

@@ -28,8 +28,10 @@ pub fn panic_handler(main: impl FnOnce() -> i32 + UnwindSafe) -> ! {
{
return;
}
if let Some(at_exit) = AT_EXIT.get() {
(at_exit)();
if is_main_thread() {
if let Some(at_exit) = AT_EXIT.get() {
(at_exit)();
}
}
eprintf!("%s crashed, please report a bug.", get_program_name());
if !is_main_thread() {

View File

@@ -103,7 +103,7 @@
},
tty_handoff::{
SCROLL_CONTENT_UP_TERMINFO_CODE, TtyHandoff, XTGETTCAP_QUERY_OS_NAME,
get_tty_protocols_active, initialize_tty_protocols, safe_deactivate_tty_protocols,
deactivate_tty_protocols, get_tty_protocols_active, initialize_tty_protocols,
},
wildcard::wildcard_has,
wutil::{fstat, perror_nix, wstat},
@@ -173,7 +173,7 @@ fn zeroed_termios() -> Termios {
pub static SHELL_MODES: LazyLock<Mutex<Termios>> = LazyLock::new(|| Mutex::new(zeroed_termios()));
/// The valid terminal modes on startup. Raw global for async-signal-safe access.
/// The valid terminal modes on startup.
static TERMINAL_MODE_ON_STARTUP: OnceLock<libc::termios> = OnceLock::new();
/// Mode we use to execute programs.
@@ -191,8 +191,8 @@ fn zeroed_termios() -> Termios {
/// Set from a signal handler.
static EXIT_SIGNAL: AtomicI32 = AtomicI32::new(0);
// 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> {
// Get the terminal mode on startup.
pub fn get_terminal_mode_on_startup() -> Option<&'static libc::termios> {
TERMINAL_MODE_ON_STARTUP.get()
}
@@ -993,7 +993,6 @@ pub fn reader_init(will_restore_foreground_pgroup: bool) {
{
Ok(modes) => {
// Save the initial terminal mode.
// Note this field is read by a signal handler, so do it atomically, with a leaked mode.
// TODO: rationalize behavior if initial tcgetattr() fails.
TERMINAL_MODE_ON_STARTUP.get_or_init(|| libc::termios::from(modes.clone()));
modes
@@ -1030,8 +1029,8 @@ pub fn reader_init(will_restore_foreground_pgroup: bool) {
}
pub fn reader_deinit(restore_foreground_pgroup: bool) {
safe_restore_term_mode();
safe_deactivate_tty_protocols();
restore_term_mode();
deactivate_tty_protocols();
if restore_foreground_pgroup {
restore_term_foreground_process_group_for_exit();
}
@@ -1040,11 +1039,11 @@ 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 safe_restore_term_mode() {
pub fn restore_term_mode() {
if !is_interactive_session() || getpgrp().as_raw() != unsafe { libc::tcgetpgrp(STDIN_FILENO) } {
return;
}
if let Some(modes) = safe_get_terminal_mode_on_startup() {
if let Some(modes) = get_terminal_mode_on_startup() {
unsafe { libc::tcsetattr(STDIN_FILENO, libc::TCSANOW, modes) };
}
}
@@ -1320,7 +1319,7 @@ pub fn read_generation_count() -> u32 {
/// The readers interrupt signal handler. Cancels all currently running blocks.
/// This is called from a signal handler!
pub fn reader_handle_sigint() {
pub fn safe_reader_handle_sigint() {
INTERRUPTED.store(SIGINT, Ordering::Relaxed);
}

View File

@@ -1,6 +1,6 @@
use crate::event::{enqueue_signal, is_signal_observed};
use crate::prelude::*;
use crate::reader::{reader_handle_sigint, safe_reader_set_exit_signal};
use crate::reader::{safe_reader_handle_sigint, safe_reader_set_exit_signal};
use crate::termsize::safe_termsize_invalidate_tty;
use crate::topic_monitor::{Generation, GenerationsList, Topic, topic_monitor_principal};
use crate::tty_handoff::safe_mark_tty_invalid;
@@ -103,7 +103,7 @@ extern "C" fn fish_signal_handler(
if !observed {
CANCELLATION_SIGNAL.store(libc::SIGINT, Ordering::Relaxed);
}
reader_handle_sigint();
safe_reader_handle_sigint();
topic_monitor_principal().post(Topic::SigHupIntTerm);
}
libc::SIGCHLD => {

View File

@@ -25,7 +25,7 @@
use std::os::fd::BorrowedFd;
use std::sync::{
OnceLock,
atomic::{AtomicBool, AtomicPtr, Ordering},
atomic::{AtomicPtr, Ordering},
};
/// Whether kitty keyboard protocol support is present in the TTY.
@@ -104,9 +104,7 @@ enum ProtocolKind {
}
// Commands to emit to enable or disable TTY protocols. Each of these contains
// the full serialized command sequence as bytes. It's structured in this awkward
// way so that we can use it from a signal handler - no need to allocate or deallocate
// as Kitty support is discovered through tty queries.
// the full serialized command sequence as bytes.
struct ProtocolBytes {
kitty_keyboard: Box<[u8]>,
other: Box<[u8]>,
@@ -115,8 +113,6 @@ struct ProtocolBytes {
}
// The combined set of TTY protocols.
// This is created once at startup and then leaked, so it may be used
// from the SIGTERM handler.
struct TtyProtocolsSet {
// TTY quirks.
quirks: TtyQuirks,
@@ -127,10 +123,8 @@ struct TtyProtocolsSet {
impl TtyProtocolsSet {
// Get commands to enable or disable TTY protocols
// and the KITTY_KEYBOARD_SUPPORTED global variable.
// THIS IS USED FROM A SIGNAL HANDLER.
fn safe_get_commands(&self, enable: bool) -> &[u8] {
let protocol = self.quirks.safe_get_supported_protocol();
fn get_commands(&self, enable: bool) -> &[u8] {
let protocol = self.quirks.get_supported_protocol();
let cmds = if enable {
&self.enablers
} else {
@@ -156,8 +150,7 @@ fn serialize_commands<'a>(cmds: impl Iterator<Item = TerminalCommand<'a>>) -> Bo
impl TtyQuirks {
// Determine which keyboard protocol.
// This is used from a signal handler.
fn safe_get_supported_protocol(&self) -> ProtocolKind {
fn get_supported_protocol(&self) -> ProtocolKind {
use TtyQuirks::{PreCsiMidnightCommander, PreKittyIterm2, Wezterm};
if *self == PreCsiMidnightCommander {
return ProtocolKind::None;
@@ -232,7 +225,6 @@ fn get_protocols(self) -> TtyProtocolsSet {
}
// The global tty protocols. This is set once at startup and not changed thereafter.
// This is an AtomicPtr and not a OnceLock, etc. so that it can be used from a signal handler.
static TTY_PROTOCOLS: AtomicPtr<TtyProtocolsSet> = AtomicPtr::new(std::ptr::null_mut());
// Get the TTY protocols, without initializing it.
@@ -241,7 +233,7 @@ fn tty_protocols() -> Option<&'static TtyProtocolsSet> {
unsafe { TTY_PROTOCOLS.load(Ordering::Acquire).as_ref() }
}
// Initialize serialized commands for enabling/disabling TTY protocols in signal handlers.
// Initialize serialized commands for enabling/disabling TTY protocols.
pub fn initialize_tty_protocols(vars: &dyn Environment) {
// Default missing query responses.
KITTY_KEYBOARD_SUPPORTED.get_or_init(|| false);
@@ -264,14 +256,13 @@ pub fn initialize_tty_protocols(vars: &dyn Environment) {
}
// A marker of the current state of the tty protocols.
static TTY_PROTOCOLS_ACTIVE: AtomicBool = AtomicBool::new(false);
static TTY_PROTOCOLS_ACTIVE: RelaxedAtomicBool = RelaxedAtomicBool::new(false);
// A marker that the tty has been closed (SIGHUP, etc) and so we should not try to write to it.
static TTY_INVALID: RelaxedAtomicBool = RelaxedAtomicBool::new(false);
// Enable or disable TTY protocols by writing the appropriate commands to the tty.
// Return true if we emitted any bytes to the tty.
// Note this does NOT intialize the TTY protocls if not already initialized.
// Note this does NOT intialize the TTY protocols if not already initialized.
fn set_tty_protocols_active(on_write: fn(), enable: bool) {
assert_is_main_thread();
// Have protocols at all? We require someone else to have initialized them.
@@ -281,11 +272,11 @@ fn set_tty_protocols_active(on_write: fn(), enable: bool) {
// Already set?
// Note we don't need atomic swaps as this is only called on the main thread.
// Also note we (logically) set and clear this even if we got SIGHUP.
if TTY_PROTOCOLS_ACTIVE.load(Ordering::Relaxed) == enable {
if TTY_PROTOCOLS_ACTIVE.load() == enable {
return;
}
if enable {
TTY_PROTOCOLS_ACTIVE.store(true, Ordering::Release);
TTY_PROTOCOLS_ACTIVE.store(true);
}
// Did we get SIGHUP?
@@ -294,15 +285,15 @@ fn set_tty_protocols_active(on_write: fn(), enable: bool) {
}
// Write the commands to the tty, ignoring errors.
let commands = protocols.safe_get_commands(enable);
let commands = protocols.get_commands(enable);
let _ = write_loop(&libc::STDOUT_FILENO, commands);
if !enable {
TTY_PROTOCOLS_ACTIVE.store(false, Ordering::Relaxed);
TTY_PROTOCOLS_ACTIVE.store(false);
}
// Flog any terminal protocol changes of interest.
let mode = if enable { "Enabling" } else { "Disabling" };
match protocols.quirks.safe_get_supported_protocol() {
match protocols.quirks.get_supported_protocol() {
ProtocolKind::KittyKeyboard => flog!(reader, mode, "kitty keyboard protocol"),
ProtocolKind::Other => flog!(reader, mode, "other extended keys"),
ProtocolKind::WorkAroundWezTerm => flog!(reader, mode, "wezterm; no modifyOtherKeys"),
@@ -313,19 +304,21 @@ fn set_tty_protocols_active(on_write: fn(), enable: bool) {
// Helper to check if TTY protocols are active.
pub fn get_tty_protocols_active() -> bool {
TTY_PROTOCOLS_ACTIVE.load(Ordering::Relaxed)
TTY_PROTOCOLS_ACTIVE.load()
}
// Called from a signal handler to deactivate TTY protocols before exiting.
// Only async-signal-safe code can be run here.
pub fn safe_deactivate_tty_protocols() {
// Deactivate TTY protocols before exiting.
pub fn deactivate_tty_protocols() {
if !cfg!(test) {
assert_is_main_thread();
}
// Safety: TTY_PROTOCOLS is never modified after initialization.
let protocols = unsafe { TTY_PROTOCOLS.load(Ordering::Acquire).as_ref() };
let Some(protocols) = protocols else {
// No protocols set, nothing to do.
return;
};
if !TTY_PROTOCOLS_ACTIVE.load(Ordering::Acquire) {
if !TTY_PROTOCOLS_ACTIVE.load() {
return;
}
@@ -334,10 +327,10 @@ pub fn safe_deactivate_tty_protocols() {
return;
}
let commands = protocols.safe_get_commands(false);
let commands = protocols.get_commands(false);
// Safety: just writing data to stdout.
let _ = write_loop(&libc::STDOUT_FILENO, commands);
TTY_PROTOCOLS_ACTIVE.store(false, Ordering::Release);
TTY_PROTOCOLS_ACTIVE.store(false);
}
// Called from a signal handler to mark the tty as invalid (e.g. SIGHUP).

View File

@@ -7,7 +7,7 @@
pub mod wcstod;
pub mod wcstoi;
use crate::{fds::BorrowedFdFile, flog, signal::SigChecker, topic_monitor::Topic};
use crate::{fds::BorrowedFdFile, flog, signal::SigChecker};
use errno::{Errno, set_errno};
use fish_util::{perror, write_to_fd};
use fish_wcstringutil::join_strings;