From 04c6442dcc23567e6176203227f258d50b6018ac Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sun, 5 Jul 2020 22:09:53 -0500 Subject: [PATCH] Allow fish_exit to run even on fish SIGHUP We were previously aborting the main event loop before calling fish_exit in the event of a SIGHUP. This patch causes the SIGHUP to be stored in a separate state variable from a regular "must exit" condition so the associated event can be fired before we terminate the loop. All streams are redirected before the event is called to prevent a SIGTTIN/SIGTTOU due to the user script reading/writing from a disposed tty. Closes #7014 --- GNUmakefile | 3 ++- src/reader.cpp | 36 +++++++++++++++++++++++++++++++----- src/reader.h | 3 +++ src/signal.cpp | 2 +- 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/GNUmakefile b/GNUmakefile index 3d0d61d65..53a7b046b 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -41,7 +41,8 @@ build/fish: build/$(BUILDFILE) build/$(BUILDFILE): build cd build; $(CMAKE) .. -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -G "$(GENERATOR)" \ - -DCMAKE_INSTALL_PREFIX="$(PREFIX)" -DCMAKE_EXPORT_COMPILE_COMMANDS=1 + -DCMAKE_INSTALL_PREFIX="$(PREFIX)" -DCMAKE_EXPORT_COMPILE_COMMANDS=1 \ + -DCMAKE_BUILD_TYPE=RelWithDebInfo build: mkdir -p build diff --git a/src/reader.cpp b/src/reader.cpp index d9e9097f0..68b305ce8 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -677,11 +677,20 @@ static void term_fix_modes(struct termios *modes) { /// Whether we should exit the current reader loop. static relaxed_atomic_bool_t s_end_current_loop{false}; +static relaxed_atomic_bool_t s_tty_redirected{false}; +static volatile sig_atomic_t s_sighup{false}; /// Whether we should exit all reader loops. This is set in response to a HUP signal and it /// latches (once set it is never cleared). This should never be reset to false. static volatile sig_atomic_t s_exit_forced{false}; -static bool should_exit() { return s_end_current_loop || s_exit_forced; } +static bool should_exit() { + if (!s_tty_redirected && s_sighup) { + redirect_tty_output(); + s_tty_redirected = true; + } + + return s_end_current_loop || s_exit_forced; +} /// Give up control of terminal. static void term_donate(outputter_t &outp) { @@ -1157,6 +1166,10 @@ void reader_force_exit() { // Beware, we may be in a signal handler. s_exit_forced = true; } +void reader_sighup() { + // Beware, we are in a signal handler + s_sighup = true; +} /// Indicates if the given command char ends paging. static bool command_ends_paging(readline_cmd_t c, bool focused_on_search_field) { @@ -2414,7 +2427,7 @@ void reader_bg_job_warning(const job_list_t &jobs) { /// This function is called when the main loop notices that end_loop has been set while in /// interactive mode. It checks if it is ok to exit. -static void handle_end_loop(const parser_t &parser) { +static void handle_end_loop(parser_t &parser) { if (!reader_exit_forced()) { for (const auto &b : parser.blocks()) { if (b.type() == block_type_t::breakpoint) { @@ -2488,7 +2501,20 @@ static int read_i(parser_t &parser) { // reader_set_buffer during evaluation. maybe_t tmp = reader_readline(0); - if (shell_is_exiting()) { + // To make fish_exit safe to use in the event of SIGHUP, first redirect the tty + // to avoid a user script triggering SIGTTIN or SIGTTOU. + if (s_sighup) { + if (!s_tty_redirected) { + redirect_tty_output(); + s_tty_redirected = true; + } + // If we call reader_force_exit first, we'll be unable to run fish_exit handlers + event_fire_generic(parser, L"fish_exit"); + s_sighup = false; + reader_force_exit(); + } + + if (should_exit()) { handle_end_loop(parser); } else if (tmp && !tmp->empty()) { const wcstring command = tmp.acquire(); @@ -2504,7 +2530,7 @@ static int read_i(parser_t &parser) { if (data->history) { data->history->resolve_pending(); } - if (shell_is_exiting()) { + if (should_exit()) { handle_end_loop(parser); } else { data->prev_end_loop = false; @@ -3554,7 +3580,7 @@ maybe_t reader_data_t::readline(int nchars_or_0) { } } - while (!rls.finished && !shell_is_exiting()) { + while (!rls.finished && !should_exit()) { // Perhaps update the termsize. This is cheap if it has not changed. update_termsize(); diff --git a/src/reader.h b/src/reader.h index 5c411d821..dd5e99a4d 100644 --- a/src/reader.h +++ b/src/reader.h @@ -121,6 +121,9 @@ int reader_read(parser_t &parser, int fd, const io_chain_t &io); /// Tell the shell whether it should exit after the currently running command finishes. void reader_set_end_loop(bool flag); +/// Mark that we encountered SIGHUP and must (soon) exit. This is invoked from a signal handler. +void reader_sighup(); + /// Mark that the reader should forcibly exit. This may be invoked from a signal handler. void reader_force_exit(); diff --git a/src/signal.cpp b/src/signal.cpp index 7eaa32a4d..367879d6f 100644 --- a/src/signal.cpp +++ b/src/signal.cpp @@ -229,7 +229,7 @@ static void fish_signal_handler(int sig, siginfo_t *info, void *context) { /// Respond to a hup signal by exiting, unless it is caught by a shellscript function, /// in which case we do nothing. if (!observed) { - reader_force_exit(); + reader_sighup(); } topic_monitor_t::principal().post(topic_t::sighupint); break;