From 0b075fce8804edbfc6b8a2a3a80dffe57ad680f0 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 29 Aug 2020 15:14:33 -0700 Subject: [PATCH] Factor the exit state to make exit handlers more explicit This adds a new type 'exit_state_t' which encapsulates where fish is in the process of exiting. This makes it explicit when fish wants to cancel "ordinary" fish script but still run exit handlers. There should be no user-visible behavior change here; this is just refactoring in preparation for the next commit. --- src/parse_execution.cpp | 2 +- src/proc.cpp | 2 +- src/reader.cpp | 32 +++++++++++++++++++++++++------- src/reader.h | 5 +++-- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 668fdf065..786e9948b 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -226,7 +226,7 @@ process_type_t parse_execution_context_t::process_type_for_command( } maybe_t parse_execution_context_t::check_end_execution() const { - if (this->cancel_signal || ctx.check_cancel() || reader_exit_forced()) { + if (this->cancel_signal || ctx.check_cancel() || check_cancel_from_fish_signal()) { return end_execution_reason_t::cancelled; } const auto &ld = parser->libdata(); diff --git a/src/proc.cpp b/src/proc.cpp index f8c8ec783..6d777e1ce 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -1012,7 +1012,7 @@ void job_t::continue_job(parser_t &parser, bool in_foreground) { if (in_foreground) { // Wait for the status of our own job to change. - while (!reader_exit_forced() && !is_stopped() && !is_completed()) { + while (!check_cancel_from_fish_signal() && !is_stopped() && !is_completed()) { process_mark_finished_children(parser, true); } } diff --git a/src/reader.cpp b/src/reader.cpp index f191c1730..e2c85cbe1 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -727,8 +727,13 @@ static void term_fix_modes(struct termios *modes) { modes->c_cc[VSTART] = disabling_char; } -/// If set, we are committed to exiting. This latches to true. -static relaxed_atomic_t s_exit_forced{false}; +/// A description of where fish is in the process of exiting. +enum class exit_state_t { + none, /// fish is not exiting. + running_handlers, /// fish intends to exit, and is running handlers like 'fish_exit'. + finished_handlers, /// fish is finished running handlers and no more fish script may be run. +}; +static relaxed_atomic_t s_exit_state{exit_state_t::none}; /// If set, SIGHUP has been received. This latches to true. /// This is set from a signal handler. @@ -798,7 +803,20 @@ static void term_steal() { termsize_container_t::shared().invalidate_tty(); } -bool reader_exit_forced() { return s_exit_forced; } +bool check_cancel_from_fish_signal() { + switch (s_exit_state) { + case exit_state_t::none: + // No reason to exit now. + return false; + case exit_state_t::running_handlers: + // We intend to exit but we want to allow these handlers to run. + return false; + case exit_state_t::finished_handlers: + // Done running exit handlers, time to exit. + return true; + } + DIE("Unreachable"); +} /// Given a command line and an autosuggestion, return the string that gets shown to the user. wcstring combine_command_and_autosuggestion(const wcstring &cmdline, @@ -2554,10 +2572,10 @@ static int read_i(parser_t &parser) { // If we are the last reader, then kill remaining jobs before exiting. if (reader_data_stack.size() == 0) { - // Once s_exit_forced is set, nothing more can be executed, - // so send the exit event now. + // Send the exit event and then commit to not executing any more fish script. + s_exit_state = exit_state_t::running_handlers; event_fire_generic(parser, L"fish_exit"); - s_exit_forced = true; + s_exit_state = exit_state_t::finished_handlers; hup_jobs(parser.jobs()); } @@ -3688,7 +3706,7 @@ maybe_t reader_data_t::readline(int nchars_or_0) { pager.clear(); } - if (!reader_exit_forced()) { + if (s_exit_state != exit_state_t::finished_handlers) { // The order of the two conditions below is important. Try to restore the mode // in all cases, but only complain if interactive. if (tcsetattr(0, TCSANOW, &old_modes) == -1 && diff --git a/src/reader.h b/src/reader.h index f56a37121..05e844c44 100644 --- a/src/reader.h +++ b/src/reader.h @@ -232,8 +232,9 @@ void reader_pop(); /// The readers interrupt signal handler. Cancels all currently running blocks. void reader_handle_sigint(); -/// This function returns true if fish is exiting by force, i.e. because stdin died. -bool reader_exit_forced(); +/// \return whether we should cancel fish script due to fish itself receiving a signal. +/// TODO: this doesn't belong in reader. +bool check_cancel_from_fish_signal(); /// Test whether the interactive reader is in search mode. bool reader_is_in_search_mode();