Support for Ctrl-C cancellation in new parser. Added tests for it too.

6 files changed, 98 insertions, 9 deletions

diff --git a/fish_tests.cpp b/fish_tests.cpp
index 29f4e830..ce61a40a 100644
--- a/fish_tests.cpp
+++ b/fish_tests.cpp
@@ -669,6 +669,66 @@ static void test_parser()
 #endif
 }
 
+/* Wait a while and then SIGINT the main thread */
+struct test_cancellation_info_t
+{
+    pthread_t thread;
+    double delay;
+};
+
+static int signal_main(test_cancellation_info_t *info)
+{
+    usleep(info->delay * 1E6);
+    pthread_kill(info->thread, SIGINT);
+    return 0;
+}
+
+static void test_1_cancellation(const wchar_t *src)
+{
+    shared_ptr<io_buffer_t> out_buff(io_buffer_t::create(false, STDOUT_FILENO));
+    const io_chain_t io_chain(out_buff);
+    test_cancellation_info_t ctx = {pthread_self(), 0.25 /* seconds */ };
+    iothread_perform(signal_main, (void (*)(test_cancellation_info_t *, int))NULL, &ctx);
+    parser_t::principal_parser().eval(src, io_chain, TOP);
+    out_buff->read();
+    if (out_buff->out_buffer_size() != 0)
+    {
+        err(L"Expected 0 bytes in out_buff, but instead found %lu bytes\n", out_buff->out_buffer_size());
+    }
+    iothread_drain_all();
+}
+
+static void test_cancellation()
+{
+    say(L"Testing Ctrl-C cancellation. If this hangs, that's a bug!");
+    
+    /* Enable fish's signal handling here. We need to make this interactive for fish to install its signal handlers */
+    proc_push_interactive(1);
+    signal_set_handlers();
+    
+    /* This tests that we can correctly ctrl-C out of certain loop constructs, and that nothing gets printed if we do */
+    
+    /* Here the command substitution is an infinite loop. echo never even gets its argument, so when we cancel we expect no output */
+    test_1_cancellation(L"echo (while true ; echo blah ; end)");
+    fprintf(stderr, ".");
+    
+    /* Nasty infinite loop that doesn't actually execute anything */
+    test_1_cancellation(L"echo (while true ; end) (while true ; end) (while true ; end)");
+    fprintf(stderr, ".");
+    
+    test_1_cancellation(L"while true ; end");
+    fprintf(stderr, ".");
+    
+    test_1_cancellation(L"for i in (whiel true ; end) ; end");
+    fprintf(stderr, ".");
+
+    
+    fprintf(stderr, "\n");
+    /* Restore signal handling */
+    proc_pop_interactive();
+    signal_reset_handlers();
+}
+
 static void test_indents()
 {
     say(L"Testing indents");
@@ -2640,12 +2700,15 @@ int main(int argc, char **argv)
     say(L"Testing low-level functionality");
     set_main_thread();
     setup_fork_guards();
-    //proc_init(); //disabling this prevents catching SIGINT
+    proc_init();
     event_init();
     function_init();
     builtin_init();
     reader_init();
     env_init();
+    
+    /* Set default signal handlers, so we can ctrl-C out of this */
+    signal_reset_handlers();
 
     if (should_test_function("highlighting")) test_highlighting();
     if (should_test_function("new_parser_ll2")) test_new_parser_ll2();
@@ -2662,6 +2725,7 @@ int main(int argc, char **argv)
     if (should_test_function("fork")) test_fork();
     if (should_test_function("iothread")) test_iothread();
     if (should_test_function("parser")) test_parser();
+    if (should_test_function("cancellation")) test_cancellation();
     if (should_test_function("indents")) test_indents();
     if (should_test_function("utils")) test_utils();
     if (should_test_function("escape_sequences")) test_escape_sequences();
diff --git a/parse_execution.cpp b/parse_execution.cpp
index bc478c05..8ca50d56 100644
--- a/parse_execution.cpp
+++ b/parse_execution.cpp
@@ -173,6 +173,10 @@ parse_execution_context_t::execution_cancellation_reason_t parse_execution_conte
     {
         return execution_cancellation_exit;
     }
+    else if (parser && parser->cancellation_requested)
+    {
+        return execution_cancellation_skip;
+    }
     else if (block && block->loop_status != LOOP_NORMAL)
     {
         /* Nasty hack - break and continue set the 'skip' flag as well as the loop status flag. */
@@ -1241,12 +1245,14 @@ parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t
     
     /* Clean up the job on failure or cancellation */
     bool populated_job = (pop_result == parse_execution_success);
-    if (! populated_job)
+    if (! populated_job || this->should_cancel_execution(associated_block))
     {
         delete j;
         j = NULL;
+        populated_job = false;
     }
     
+    
     /* Store time it took to 'parse' the command */
     if (do_profile)
     {
diff --git a/parser.cpp b/parser.cpp
index 6f500817..a96f72b1 100644
--- a/parser.cpp
+++ b/parser.cpp
@@ -315,6 +315,7 @@ parser_t::parser_t(enum parser_type_t type, bool errors) :
     show_errors(errors),
     error_code(0),
     err_pos(0),
+    cancellation_requested(false),
     current_tokenizer(NULL),
     current_tokenizer_pos(0),
     job_start_pos(0),
@@ -343,6 +344,8 @@ void parser_t::skip_all_blocks(void)
     /* Tell all blocks to skip */
     if (s_principal_parser)
     {
+        s_principal_parser->cancellation_requested = true;
+
         //write(2, "Cancelling blocks\n", strlen("Cancelling blocks\n"));
         for (size_t i=0; i < s_principal_parser->block_count(); i++)
         {
@@ -2614,7 +2617,20 @@ int parser_t::eval_block_node(node_offset_t node_idx, const io_chain_t &io, enum
     assert(ctx != NULL);
     
     CHECK_BLOCK(1);
-
+    
+    /* 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 (! block_stack.empty())
+        {
+            return 1;
+        }
+        else
+        {
+            this->cancellation_requested = false;
+        }
+    }
+    
     /* Only certain blocks are allowed */
     if ((block_type != TOP) &&
             (block_type != SUBST))
@@ -2662,6 +2678,7 @@ int parser_t::eval(const wcstring &cmd_str, const io_chain_t &io, enum block_typ
 
     if (parser_use_ast())
         return this->eval_new_parser(cmd_str, io, block_type);
+    
     const wchar_t * const cmd = cmd_str.c_str();
     size_t forbid_count;
     int code;
diff --git a/parser.h b/parser.h
index f013a3b9..0c8c7334 100644
--- a/parser.h
+++ b/parser.h
@@ -287,6 +287,9 @@ private:
     /** Position of last error */
     int err_pos;
     
+    /** Indication that we should skip all blocks */
+    bool cancellation_requested;
+    
     /** Stack of execution contexts. We own these pointers and must delete them */
     std::vector<parse_execution_context_t *> execution_contexts;
 
diff --git a/proc.cpp b/proc.cpp
index 4a25b451..4a1bfd5b 100644
--- a/proc.cpp
+++ b/proc.cpp
@@ -136,7 +136,8 @@ static bool proc_had_barrier = false;
 int get_is_interactive(void)
 {
     ASSERT_IS_MAIN_THREAD();
-    // The tests leave is_interactive as -1, which is interpreted as true. So let's have them default to false.
+    /* is_interactive is initialized to -1; ensure someone has popped/pushed it before then */
+    assert(is_interactive >= 0);
     return is_interactive > 0;
 }
 
diff --git a/reader.cpp b/reader.cpp
index f5ae62f4..b03b84a8 100644
--- a/reader.cpp
+++ b/reader.cpp
@@ -2068,11 +2068,9 @@ static void reader_interactive_destroy()
 
 void reader_sanity_check()
 {
-    if (get_is_interactive())
+    /* Note: 'data' is non-null if we are interactive, except in the testing environment */
+    if (get_is_interactive() && data != NULL)
     {
-        if (!data)
-            sanity_lose();
-
         if (!(data->buff_pos <= data->command_length()))
             sanity_lose();
 
@@ -2739,7 +2737,7 @@ static void reader_super_highlight_me_plenty(size_t match_highlight_pos)
 bool shell_is_exiting()
 {
     if (get_is_interactive())
-        return job_list_is_empty() && data->end_loop;
+        return job_list_is_empty() && data != NULL && data->end_loop;
     else
         return end_loop;
 }