Migrate the "are you sure you want to exit" logic from parse_execution to exec

This feels more like the sort of logic that should live in the point where
jobs are executed, instead of where jobs are created from parse trees.

src/exec.cpp

@@ -884,6 +884,30 @@ static process_t *get_deferred_process(const shared_ptr<job_t> &j) {
     return nullptr;
 }
 
+// Given that we are about to execute an exec() call, check if the parser is interactive and there
+// are extant background jobs. If so, warn the user and do not exec().
+// \return true if we should allow exec, false to disallow it.
+static bool allow_exec_with_background_jobs(parser_t &parser) {
+    // If we're not interactive, we cannot warn.
+    if (!parser.is_interactive()) return true;
+
+    // Construct the list of running background jobs.
+    job_list_t bgs = jobs_requiring_warning_on_exit(parser);
+    if (bgs.empty()) return true;
+
+    // Compare run counts, so we only warn once.
+    uint64_t current_run_count = reader_run_count();
+    uint64_t &last_exec_run_count = parser.libdata().last_exec_run_counter;
+    if (isatty(STDIN_FILENO) && current_run_count - 1 != last_exec_run_count) {
+        print_exit_warning_for_jobs(bgs);
+        last_exec_run_count = current_run_count;
+        return false;
+    } else {
+        hup_background_jobs(parser);
+        return true;
+    }
+}
+
 bool exec_job(parser_t &parser, const shared_ptr<job_t> &j, const job_lineage_t &lineage) {
     assert(j && "null job_t passed to exec_job!");
 
@@ -930,6 +954,11 @@ bool exec_job(parser_t &parser, const shared_ptr<job_t> &j, const job_lineage_t
 
     // Handle an exec call.
     if (j->processes.front()->type == process_type_t::exec) {
+        // If we are interactive, perhaps disallow exec if there are background jobs.
+        if (!allow_exec_with_background_jobs(parser)) {
+            return false;
+        }
+
         internal_exec(parser.vars(), j.get(), lineage.block_io);
         // internal_exec only returns if it failed to set up redirections.
         // In case of an successful exec, this code is not reached.

src/parse_execution.cpp

@@ -749,32 +749,6 @@ end_execution_reason_t parse_execution_context_t::populate_plain_process(
     // Determine the process type.
     enum process_type_t process_type = process_type_for_command(statement, cmd);
 
-    // Protect against exec with background processes running
-    if (process_type == process_type_t::exec && parser->is_interactive()) {
-        bool have_bg = false;
-        for (const auto &bg : parser->jobs()) {
-            // The assumption here is that if it is a foreground job,
-            // it's related to us.
-            // This stops us from asking if we're doing `exec` inside a function.
-            if (!bg->is_completed() && !bg->is_foreground()) {
-                have_bg = true;
-                break;
-            }
-        }
-
-        if (have_bg) {
-            uint64_t current_run_count = reader_run_count();
-            uint64_t &last_exec_run_count = parser->libdata().last_exec_run_counter;
-            if (isatty(STDIN_FILENO) && current_run_count - 1 != last_exec_run_count) {
-                reader_bg_job_warning(*parser);
-                last_exec_run_count = current_run_count;
-                return end_execution_reason_t::error;
-            } else {
-                hup_background_jobs(*parser);
-            }
-        }
-    }
-
     wcstring path_to_external_command;
     if (process_type == process_type_t::external || process_type == process_type_t::exec) {
         // Determine the actual command. This may be an implicit cd.

src/proc.cpp

@@ -210,6 +210,27 @@ static int64_t next_proc_id() {
 
 internal_proc_t::internal_proc_t() : internal_proc_id_(next_proc_id()) {}
 
+job_list_t jobs_requiring_warning_on_exit(const parser_t &parser) {
+    job_list_t result;
+    for (const auto &job : parser.jobs()) {
+        if (!job->is_foreground() && job->is_constructed() && !job->is_completed()) {
+            result.push_back(job);
+        }
+    }
+    return result;
+}
+
+void print_exit_warning_for_jobs(const job_list_t &jobs) {
+    fputws(_(L"There are still jobs active:\n"), stdout);
+    fputws(_(L"\n   PID  Command\n"), stdout);
+    for (const auto &j : jobs) {
+        fwprintf(stdout, L"%6d  %ls\n", j->processes[0]->pid, j->command_wcstr());
+    }
+    fputws(L"\n", stdout);
+    fputws(_(L"A second attempt to exit will terminate them.\n"), stdout);
+    fputws(_(L"Use 'disown PID' to remove jobs from the list without terminating them.\n"), stdout);
+}
+
 void job_mark_process_as_failed(const std::shared_ptr<job_t> &job, const process_t *failed_proc) {
     // The given process failed to even lift off (e.g. posix_spawn failed) and so doesn't have a
     // valid pid. Mark it and everything after it as dead.

src/proc.h

@@ -553,6 +553,14 @@ void set_job_control_mode(job_control_t mode);
 class parser_t;
 bool job_reap(parser_t &parser, bool interactive);
 
+/// \return the list of background jobs which we should warn the user about, if the user attempts to
+/// exit. An empty result (common) means no such jobs.
+job_list_t jobs_requiring_warning_on_exit(const parser_t &parser);
+
+/// Print the exit warning for the given jobs, which should have been obtained via
+/// jobs_requiring_warning_on_exit().
+void print_exit_warning_for_jobs(const job_list_t &jobs);
+
 /// Mark a process as failed to execute (and therefore completed).
 void job_mark_process_as_failed(const std::shared_ptr<job_t> &job, const process_t *failed_proc);
 

src/reader.cpp

@@ -2341,14 +2341,12 @@ void reader_import_history_if_necessary() {
 
 bool shell_is_exiting() { return should_exit(); }
 
-void reader_bg_job_warning(const parser_t &parser) {
+void reader_bg_job_warning(const job_list_t &jobs) {
     std::fputws(_(L"There are still jobs active:\n"), stdout);
     std::fputws(_(L"\n   PID  Command\n"), stdout);
 
-    for (const auto &j : parser.jobs()) {
-        if (!j->is_completed()) {
-            std::fwprintf(stdout, L"%6d  %ls\n", j->processes[0]->pid, j->command_wcstr());
-        }
+    for (const auto &j : jobs) {
+        std::fwprintf(stdout, L"%6d  %ls\n", j->processes[0]->pid, j->command_wcstr());
     }
     fputws(L"\n", stdout);
     fputws(_(L"A second attempt to exit will terminate them.\n"), stdout);
@@ -2367,17 +2365,10 @@ static void handle_end_loop(const parser_t &parser) {
             }
         }
 
-        bool bg_jobs = false;
-        for (const auto &j : parser.jobs()) {
-            if (!j->is_completed()) {
-                bg_jobs = true;
-                break;
-            }
-        }
-
         reader_data_t *data = current_data();
-        if (!data->prev_end_loop && bg_jobs) {
-            reader_bg_job_warning(parser);
+        auto bg_jobs = jobs_requiring_warning_on_exit(parser);
+        if (!data->prev_end_loop && !bg_jobs.empty()) {
+            print_exit_warning_for_jobs(bg_jobs);
             reader_set_end_loop(false);
             data->prev_end_loop = true;
             return;

src/reader.h

@@ -283,9 +283,6 @@ wcstring completion_apply_to_command_line(const wcstring &val_str, complete_flag
                                           const wcstring &command_line, size_t *inout_cursor_pos,
                                           bool append_only);
 
-/// Print warning with list of backgrounded jobs
-void reader_bg_job_warning(const parser_t &parser);
-
 /// Return the current interactive reads loop count. Useful for determining how many commands have
 /// been executed between invocations of code.
 uint64_t reader_run_count();