Make SIGTERM handler async-signal-safe again

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);
+        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);

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);
 }

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;

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() {

src/panic.rs

@@ -13,7 +13,7 @@ use crate::{
     threads::{asan_maybe_exit, is_main_thread},
 };
 
-pub static AT_EXIT: OnceCell<Box<dyn Fn() + Send + Sync>> = OnceCell::new();
+pub static AT_EXIT: OnceCell<Box<dyn Fn(bool) + Send + Sync>> = 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)

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;
                     }
                     _ => {

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");

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 {

src/tests/mod.rs

@@ -111,7 +111,7 @@ pub mod prelude {
         });
         reader_init(false);
         ScopeGuard::new((), |()| {
-            reader_deinit(false);
+            reader_deinit(false, false);
         })
     }