diff --git a/fish_tests.cpp b/fish_tests.cpp index 29f4e830b..ce61a40af 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 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 bc478c05d..8ca50d565 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 6f5008174..a96f72b1c 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 f013a3b92..0c8c7334a 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 execution_contexts; diff --git a/proc.cpp b/proc.cpp index 4a25b4517..4a1bfd5b5 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 f5ae62f48..b03b84a80 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; }