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
This commit is contained in:
Mahmoud Al-Qudsi 2020-07-05 22:09:53 -05:00
parent 791c4fb1dd
commit 04c6442dcc
4 changed files with 37 additions and 7 deletions

View file

@ -41,7 +41,8 @@ build/fish: build/$(BUILDFILE)
build/$(BUILDFILE): build build/$(BUILDFILE): build
cd build; $(CMAKE) .. -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -G "$(GENERATOR)" \ 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: build:
mkdir -p build mkdir -p build

View file

@ -677,11 +677,20 @@ static void term_fix_modes(struct termios *modes) {
/// Whether we should exit the current reader loop. /// Whether we should exit the current reader loop.
static relaxed_atomic_bool_t s_end_current_loop{false}; 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 /// 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. /// latches (once set it is never cleared). This should never be reset to false.
static volatile sig_atomic_t s_exit_forced{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. /// Give up control of terminal.
static void term_donate(outputter_t &outp) { static void term_donate(outputter_t &outp) {
@ -1157,6 +1166,10 @@ void reader_force_exit() {
// Beware, we may be in a signal handler. // Beware, we may be in a signal handler.
s_exit_forced = true; 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. /// Indicates if the given command char ends paging.
static bool command_ends_paging(readline_cmd_t c, bool focused_on_search_field) { 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 /// 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. /// 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()) { if (!reader_exit_forced()) {
for (const auto &b : parser.blocks()) { for (const auto &b : parser.blocks()) {
if (b.type() == block_type_t::breakpoint) { if (b.type() == block_type_t::breakpoint) {
@ -2488,7 +2501,20 @@ static int read_i(parser_t &parser) {
// reader_set_buffer during evaluation. // reader_set_buffer during evaluation.
maybe_t<wcstring> tmp = reader_readline(0); maybe_t<wcstring> 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); handle_end_loop(parser);
} else if (tmp && !tmp->empty()) { } else if (tmp && !tmp->empty()) {
const wcstring command = tmp.acquire(); const wcstring command = tmp.acquire();
@ -2504,7 +2530,7 @@ static int read_i(parser_t &parser) {
if (data->history) { if (data->history) {
data->history->resolve_pending(); data->history->resolve_pending();
} }
if (shell_is_exiting()) { if (should_exit()) {
handle_end_loop(parser); handle_end_loop(parser);
} else { } else {
data->prev_end_loop = false; data->prev_end_loop = false;
@ -3554,7 +3580,7 @@ maybe_t<wcstring> 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. // Perhaps update the termsize. This is cheap if it has not changed.
update_termsize(); update_termsize();

View file

@ -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. /// Tell the shell whether it should exit after the currently running command finishes.
void reader_set_end_loop(bool flag); 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. /// Mark that the reader should forcibly exit. This may be invoked from a signal handler.
void reader_force_exit(); void reader_force_exit();

View file

@ -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, /// Respond to a hup signal by exiting, unless it is caught by a shellscript function,
/// in which case we do nothing. /// in which case we do nothing.
if (!observed) { if (!observed) {
reader_force_exit(); reader_sighup();
} }
topic_monitor_t::principal().post(topic_t::sighupint); topic_monitor_t::principal().post(topic_t::sighupint);
break; break;