Be more disciplined about cancellation signals

Rather than storing a "should cancel" flag in the parser, store the
actual signal which triggered cancellation.

src/parse_execution.cpp

@@ -189,7 +189,7 @@ maybe_t<eval_result_t> parse_execution_context_t::check_end_execution() const {
     if (shell_is_exiting()) {
         return eval_result_t::cancelled;
     }
-    if (parser && parser->cancellation_requested) {
+    if (parser && parser->cancellation_signal) {
         return eval_result_t::cancelled;
     }
     const auto &ld = parser->libdata();
@@ -643,7 +643,7 @@ eval_result_t parse_execution_context_t::report_error(const parse_node_t &node,
 }
 
 eval_result_t parse_execution_context_t::report_errors(const parse_error_list_t &error_list) const {
-    if (!parser->cancellation_requested) {
+    if (!parser->cancellation_signal) {
         if (error_list.empty()) {
             FLOG(error, L"Error reported but no error text found.");
         }

src/parser.cpp

@@ -102,10 +102,9 @@ parser_t &parser_t::principal_parser() {
     return *principal;
 }
 
-void parser_t::skip_all_blocks() {
+void parser_t::cancel_requested(int sig) {
-    // Tell all blocks to skip.
+    assert(sig != 0 && "Signal must not be 0");
-    // This may be called from a signal handler!
+    principal->cancellation_signal = sig;
-    principal->cancellation_requested = true;
 }
 
 // Given a new-allocated block, push it onto our block list, acquiring ownership.
@@ -648,11 +647,11 @@ eval_result_t parser_t::eval_node(const parsed_source_ref_t &ps, tnode_t<T> node
     // Handle cancellation requests. If our block stack is currently empty, then we already did
     // successfully cancel (or there was nothing to cancel); clear the flag. If our block stack is
     // not empty, we are still in the process of cancelling; refuse to evaluate anything.
-    if (this->cancellation_requested) {
+    if (this->cancellation_signal) {
         if (!block_list.empty()) {
             return eval_result_t::cancelled;
         }
-        this->cancellation_requested = false;
+        this->cancellation_signal = 0;
     }
 
     // Only certain blocks are allowed.

src/parser.h

@@ -201,8 +201,8 @@ class parser_t : public std::enable_shared_from_this<parser_t> {
     friend class parse_execution_context_t;
 
    private:
-    /// Indication that we should skip all blocks.
+    /// If not zero, the signal triggering cancellation.
-    volatile sig_atomic_t cancellation_requested = false;
+    volatile sig_atomic_t cancellation_signal = 0;
     /// The current execution context.
     std::unique_ptr<parse_execution_context_t> execution_context;
     /// The jobs associated with this parser.
@@ -248,9 +248,8 @@ class parser_t : public std::enable_shared_from_this<parser_t> {
     /// Get the "principal" parser, whatever that is.
     static parser_t &principal_parser();
 
-    /// Indicates that execution of all blocks in the principal parser should stop. This is called
+    /// Indicates that we should stop execution due to the given signal.
-    /// from signal handlers!
+    static void cancel_requested(int sig);
-    static void skip_all_blocks();
 
     /// Global event blocks.
     event_blockage_list_t global_event_blocks;

src/proc.cpp

@@ -249,7 +249,7 @@ static void handle_child_status(process_t *proc, proc_status_t status) {
             if (session_interactivity() != session_interactivity_t::not_interactive) {
                 // In an interactive session, tell the principal parser to skip all blocks we're
                 // executing so control-C returns control to the user.
-                parser_t::skip_all_blocks();
+                parser_t::cancel_requested(sig);
             } else {
                 // Deliver the SIGINT or SIGQUIT signal to ourself since we're not interactive.
                 struct sigaction act;

src/reader.cpp

@@ -489,7 +489,7 @@ class reader_data_t : public std::enable_shared_from_this<reader_data_t> {
     void remove_backward();
 };
 
-/// This variable is set to true by the signal handler when ^C is pressed.
+/// This variable is set to a signal by the signal handler when ^C is pressed.
 static volatile sig_atomic_t interrupted = 0;
 
 // Prototypes for a bunch of functions defined later on.
@@ -686,8 +686,8 @@ void reader_data_t::kill(editable_line_t *el, size_t begin_idx, size_t length, i
 
 // This is called from a signal handler!
 void reader_handle_sigint() {
-    parser_t::skip_all_blocks();
+    parser_t::cancel_requested(SIGINT);
-    interrupted = 1;
+    interrupted = SIGINT;
 }
 
 /// Make sure buffers are large enough to hold the current string length.
@@ -849,12 +849,12 @@ bool reader_test_should_cancel() {
     }
 }
 
-bool reader_test_and_clear_interrupted() {
+int reader_test_and_clear_interrupted() {
     int res = interrupted;
     if (res) {
         interrupted = 0;
     }
-    return res != 0;
+    return res;
 }
 
 void reader_write_title(const wcstring &cmd, parser_t &parser, bool reset_cursor_position) {
@@ -3411,7 +3411,7 @@ int reader_reading_interrupted() {
     reader_data_t *data = current_data_or_null();
     if (res && data && data->exit_on_interrupt) {
         reader_set_end_loop(true);
-        parser_t::skip_all_blocks();
+        parser_t::cancel_requested(res);
         // We handled the interrupt ourselves, our caller doesn't need to handle it.
         return 0;
     }

src/reader.h

@@ -121,8 +121,8 @@ bool reader_get_selection(size_t *start, size_t *len);
 bool reader_test_should_cancel();
 
 /// Return the value of the interrupted flag, which is set by the sigint handler, and clear it if it
-/// was set.
+/// was set. In practice this will return 0 or SIGINT.
-bool reader_test_and_clear_interrupted();
+int reader_test_and_clear_interrupted();
 
 /// Clear the interrupted flag unconditionally without handling anything. The flag could have been
 /// set e.g. when an interrupt arrived just as we were ending an earlier \c reader_readline