diff --git a/src/builtin.cpp b/src/builtin.cpp index 6ee68a005..9c6f33609 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -3033,15 +3033,14 @@ static int builtin_break_continue(parser_t &parser, io_streams_t &streams, wchar return STATUS_BUILTIN_ERROR; } - // Skip blocks interior to the loop. + // Skip blocks interior to the loop (but not the loop itself) size_t block_idx = loop_idx; while (block_idx--) { parser.block_at_index(block_idx)->skip = true; } - /* Skip the loop itself */ + // Mark the loop's status block_t *loop_block = parser.block_at_index(loop_idx); - loop_block->skip = true; loop_block->loop_status = is_break ? LOOP_BREAK : LOOP_CONTINUE; return STATUS_BUILTIN_OK; } @@ -3098,12 +3097,11 @@ static int builtin_return(parser_t &parser, io_streams_t &streams, wchar_t **arg return STATUS_BUILTIN_ERROR; } - // Skip everything up to (and then including) the function block. - for (size_t i = 0; i < function_block_idx; i++) { + // Skip everything up to and including the function block. + for (size_t i = 0; i <= function_block_idx; i++) { block_t *b = parser.block_at_index(i); b->skip = true; } - parser.block_at_index(function_block_idx)->skip = true; return status; } diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index e5d6d9b11..6fb542841 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -769,6 +769,7 @@ static void test_cancellation() { // Nasty infinite loop that doesn't actually execute anything. test_1_cancellation(L"echo (while true ; end) (while true ; end) (while true ; end)"); test_1_cancellation(L"while true ; end"); + test_1_cancellation(L"while true ; echo nothing > /dev/null; end"); test_1_cancellation(L"for i in (while true ; end) ; end"); // Restore signal handling. diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 3b10d2e08..72e0b72df 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -220,13 +220,12 @@ parse_execution_context_t::cancellation_reason(const block_t *block) const { if (parser && parser->cancellation_requested) { return execution_cancellation_skip; } - if (block && block->loop_status != LOOP_NORMAL) { - // Nasty hack - break and continue set the 'skip' flag as well as the loop status flag. - return execution_cancellation_loop_control; - } if (block && block->skip) { return execution_cancellation_skip; } + if (block && block->loop_status != LOOP_NORMAL) { + return execution_cancellation_loop_control; + } return execution_cancellation_none; } @@ -485,7 +484,6 @@ parse_execution_result_t parse_execution_context_t::run_for_statement( const wcstring &val = argument_sequence.at(i); env_set(for_var_name, val.c_str(), ENV_LOCAL); fb->loop_status = LOOP_NORMAL; - fb->skip = 0; this->run_job_list(block_contents, fb); @@ -494,7 +492,6 @@ parse_execution_result_t parse_execution_context_t::run_for_statement( if (fb->loop_status == LOOP_CONTINUE) { // Reset the loop state. fb->loop_status = LOOP_NORMAL; - fb->skip = false; continue; } else if (fb->loop_status == LOOP_BREAK) { break; @@ -656,7 +653,6 @@ parse_execution_result_t parse_execution_context_t::run_while_statement( if (wb->loop_status == LOOP_CONTINUE) { // Reset the loop state. wb->loop_status = LOOP_NORMAL; - wb->skip = false; continue; } else if (wb->loop_status == LOOP_BREAK) { break; diff --git a/src/parser.cpp b/src/parser.cpp index 0a7c77762..d50d9ecec 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -101,17 +101,13 @@ static wcstring user_presentable_path(const wcstring &path) { parser_t::parser_t() : cancellation_requested(false), is_within_fish_initialization(false) {} -/// A pointer to the principal parser (which is a static local). -static parser_t *s_principal_parser = NULL; + +static parser_t s_principal_parser; parser_t &parser_t::principal_parser(void) { ASSERT_IS_NOT_FORKED_CHILD(); ASSERT_IS_MAIN_THREAD(); - static parser_t parser; - if (!s_principal_parser) { - s_principal_parser = &parser; - } - return parser; + return s_principal_parser; } void parser_t::set_is_within_fish_initialization(bool flag) { @@ -120,14 +116,8 @@ void parser_t::set_is_within_fish_initialization(bool flag) { 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++) { - s_principal_parser->block_at_index(i)->skip = true; - } - } + // This may be called from a signal handler! + s_principal_parser.cancellation_requested = true; } void parser_t::push_block(block_t *new_current) { @@ -139,22 +129,17 @@ void parser_t::push_block(block_t *new_current) { new_current->src_filename = intern(filename); } + // New blocks should be skipped if the outer block is skipped, except TOP and SUBST block, which + // open up new environments. const block_t *old_current = this->current_block(); - if (old_current && old_current->skip) { - new_current->skip = true; - } - - // New blocks should be skipped if the outer block is skipped, except TOP ans SUBST block, which - // open up new environments. Fake blocks should always be skipped. Rather complicated... :-( - new_current->skip = old_current ? old_current->skip : 0; + new_current->skip = old_current && old_current->skip; // Type TOP and SUBST are never skipped. if (type == TOP || type == SUBST) { - new_current->skip = 0; + new_current->skip = false; } - - new_current->job = 0; + new_current->job = nullptr; new_current->loop_status = LOOP_NORMAL; this->block_stack.push_back(new_current); @@ -782,7 +767,7 @@ void parser_t::get_backtrace(const wcstring &src, const parse_error_list_t &erro block_t::block_t(block_type_t t) : block_type(t), - skip(), + skip(false), tok_pos(), node_offset(NODE_OFFSET_INVALID), loop_status(LOOP_NORMAL), diff --git a/src/parser.h b/src/parser.h index 29d7f9452..380c70fcb 100644 --- a/src/parser.h +++ b/src/parser.h @@ -178,7 +178,7 @@ class parser_t { private: /// Indication that we should skip all blocks. - bool cancellation_requested; + volatile sig_atomic_t cancellation_requested; /// Indicates that we are within the process of initializing fish. bool is_within_fish_initialization; /// Stack of execution contexts. We own these pointers and must delete them.