diff --git a/src/builtin_wait.cpp b/src/builtin_wait.cpp index 6df0d3abf..5a2f9b291 100644 --- a/src/builtin_wait.cpp +++ b/src/builtin_wait.cpp @@ -64,14 +64,14 @@ static bool any_jobs_finished(size_t jobs_len) { return false; } -static int wait_for_backgrounds(bool any_flag) { +static int wait_for_backgrounds(parser_t &parser, bool any_flag) { size_t jobs_len = jobs().size(); while ((!any_flag && !all_jobs_finished()) || (any_flag && !any_jobs_finished(jobs_len))) { if (reader_test_interrupted()) { return 128 + SIGINT; } - proc_wait_any(); + proc_wait_any(parser); } return 0; } @@ -104,13 +104,14 @@ static bool any_specified_jobs_finished(const std::vector &ids) { return false; } -static int wait_for_backgrounds_specified(const std::vector &ids, bool any_flag) { +static int wait_for_backgrounds_specified(parser_t &parser, const std::vector &ids, + bool any_flag) { while ((!any_flag && !all_specified_jobs_finished(ids)) || (any_flag && !any_specified_jobs_finished(ids))) { if (reader_test_interrupted()) { return 128 + SIGINT; } - proc_wait_any(); + proc_wait_any(parser); } return 0; } @@ -204,7 +205,7 @@ int builtin_wait(parser_t &parser, io_streams_t &streams, wchar_t **argv) { if (w.woptind == argc) { // no jobs specified - retval = wait_for_backgrounds(any_flag); + retval = wait_for_backgrounds(parser, any_flag); } else { // jobs specified std::vector waited_job_ids; @@ -236,7 +237,7 @@ int builtin_wait(parser_t &parser, io_streams_t &streams, wchar_t **argv) { if (waited_job_ids.empty()) return STATUS_INVALID_ARGS; - retval = wait_for_backgrounds_specified(waited_job_ids, any_flag); + retval = wait_for_backgrounds_specified(parser, waited_job_ids, any_flag); } return retval; diff --git a/src/exec.cpp b/src/exec.cpp index d3d604c91..414665927 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -286,7 +286,7 @@ void internal_exec_helper(parser_t &parser, parsed_source_ref_t parsed_source, t morphed_chain.clear(); io_cleanup_fds(opened_fds); - job_reap(false); + job_reap(parser, false); } // Returns whether we can use posix spawn for a given process in a given job. diff --git a/src/input.cpp b/src/input.cpp index 9a685d952..68d026e36 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -253,7 +253,8 @@ static maybe_t interrupt_handler() { // Fire any pending events. event_fire_delayed(); // Reap stray processes, including printing exit status messages. - if (job_reap(true)) reader_repaint_needed(); + // TODO: shouldn't need this parser here. + if (job_reap(parser_t::principal_parser(), true)) reader_repaint_needed(); // Tell the reader an event occured. if (reader_reading_interrupted()) { auto vintr = shell_modes.c_cc[VINTR]; diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index bb334f238..abdb53497 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -1294,7 +1294,7 @@ parse_execution_result_t parse_execution_context_t::run_1_job(tnode_t jo profile_item->skipped = !populated_job; } - job_reap(false); // clean up jobs + job_reap(*parser, false); // clean up jobs return populated_job ? parse_execution_success : parse_execution_errored; } diff --git a/src/parser.cpp b/src/parser.cpp index 1f59adc7c..c07fcdc1c 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -670,7 +670,7 @@ int parser_t::eval_node(parsed_source_ref_t ps, tnode_t node, const io_chain_ return 1; } - job_reap(false); // not sure why we reap jobs here + job_reap(*this, false); // not sure why we reap jobs here // Start it up scope_block_t *scope_block = this->push_block(block_type); @@ -683,7 +683,7 @@ int parser_t::eval_node(parsed_source_ref_t ps, tnode_t node, const io_chain_ exc.restore(); this->pop_block(scope_block); - job_reap(false); // reap again + job_reap(*this, false); // reap again return result; } diff --git a/src/parser.h b/src/parser.h index 0796e388a..dce8cb3d5 100644 --- a/src/parser.h +++ b/src/parser.h @@ -157,6 +157,9 @@ struct library_data_t { /// Number of recursive calls to builtin_complete(). uint32_t builtin_complete_recursion_level{0}; + + /// Whether we are currently cleaning processes. + bool is_cleaning_procs{false}; }; class parser_t : public std::enable_shared_from_this { diff --git a/src/proc.cpp b/src/proc.cpp index 97dfd919a..c3f3b8492 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -551,16 +551,17 @@ static bool job_wants_message(const shared_ptr &j, bool print_for_foregro /// Remove completed jobs from the job list, printing status messages as appropriate. /// \return whether something was printed. -static bool process_clean_after_marking(bool allow_interactive) { +static bool process_clean_after_marking(parser_t &parser, bool allow_interactive) { ASSERT_IS_MAIN_THREAD(); bool printed = false; // This function may fire an event handler, we do not want to call ourselves recursively (to // avoid infinite recursion). - static std::atomic locked { false }; - if (locked.exchange(true, std::memory_order::memory_order_relaxed)) { + if (parser.libdata().is_cleaning_procs) { return false; } + parser.libdata().is_cleaning_procs = true; + const cleanup_t cleanup([&] { parser.libdata().is_cleaning_procs = false; }); // This may be invoked in an exit handler, after the TERM has been torn down // Don't try to print in that case (#3222) @@ -628,18 +629,17 @@ static bool process_clean_after_marking(bool allow_interactive) { fflush(stdout); } - locked = false; return printed; } -bool job_reap(bool allow_interactive) { +bool job_reap(parser_t &parser, bool allow_interactive) { ASSERT_IS_MAIN_THREAD(); process_mark_finished_children(false); // Preserve the exit status. auto saved_statuses = proc_get_last_statuses(); - bool printed = process_clean_after_marking(allow_interactive); + bool printed = process_clean_after_marking(parser, allow_interactive); // Restore the exit status. proc_set_last_statuses(std::move(saved_statuses)); @@ -970,10 +970,10 @@ void proc_pop_interactive() { if (is_interactive != old) signal_set_handlers(); } -void proc_wait_any() { +void proc_wait_any(parser_t &parser) { ASSERT_IS_MAIN_THREAD(); process_mark_finished_children(true /* block_ok */); - process_clean_after_marking(is_interactive); + process_clean_after_marking(parser, is_interactive); } void hup_background_jobs() { diff --git a/src/proc.h b/src/proc.h index e74610e93..573fd8aab 100644 --- a/src/proc.h +++ b/src/proc.h @@ -483,9 +483,10 @@ int proc_get_last_status(); statuses_t proc_get_last_statuses(); /// Notify the user about stopped or terminated jobs, and delete completed jobs from the job list. -/// If \p interactive is set, allow reaping interactive jobs; otherwise skip them. +/// If \p interactive is set, allow removing interactive jobs; otherwise skip them. /// \return whether text was printed to stdout. -bool job_reap(bool interactive); +class parser_t; +bool job_reap(parser_t &parser, bool interactive); /// Mark a process as failed to execute (and therefore completed). void job_mark_process_as_failed(const std::shared_ptr &job, const process_t *p); @@ -518,7 +519,7 @@ void proc_push_interactive(int value); void proc_pop_interactive(); /// Wait for any process finishing, or receipt of a signal. -void proc_wait_any(); +void proc_wait_any(parser_t &parser); /// Set and get whether we are in initialization. // Hackish. In order to correctly report the origin of code with no associated file, we need to diff --git a/src/reader.cpp b/src/reader.cpp index fbf784a4d..34090cab0 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -1961,7 +1961,7 @@ void reader_run_command(parser_t &parser, const wcstring &cmd) { gettimeofday(&time_before, NULL); parser.eval(cmd, io_chain_t(), TOP); - job_reap(true); + job_reap(parser, true); gettimeofday(&time_after, NULL);