diff --git a/src/exec.cpp b/src/exec.cpp index b201ab5ba..69c38f7aa 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -499,6 +499,19 @@ static bool exec_internal_builtin_proc(parser_t &parser, job_t *j, process_t *p, return true; // "success" } +void on_process_created(job_t *j, pid_t child_pid) { + // We only need to do this the first time a child is forked/spawned + if (j->pgid != -2) { + return; + } + + if (j->get_flag(JOB_CONTROL)) { + j->pgid = child_pid; + } else { + j->pgid = getpgrp(); + } +} + void exec_job(parser_t &parser, job_t *j) { pid_t pid = 0; @@ -587,7 +600,9 @@ void exec_job(parser_t &parser, job_t *j) { // Parent debug(2, L"Fork #%d, pid %d: keepalive fork for '%ls'", g_fork_count, keepalive.pid, j->command_wcstr()); + on_process_created(j, keepalive.pid); set_child_group(j, keepalive.pid); + maybe_assign_terminal(j); } } @@ -732,6 +747,8 @@ void exec_job(parser_t &parser, job_t *j) { // This is the io_streams we pass to internal builtins. std::unique_ptr builtin_io_streams(new io_streams_t(stdout_read_limit)); + // We fork in several different places. Each time the same code must be executed, so unify + // it all here. auto do_fork = [&j, &p, &pid, &exec_error, &process_net_io_chain, &child_forked](bool drain_threads, const char *fork_type, std::function child_action) -> bool { @@ -744,22 +761,25 @@ void exec_job(parser_t &parser, job_t *j) { setup_child_process(p, process_net_io_chain); child_action(); DIE("Child process returned control to do_fork lambda!"); - } else { - if (pid < 0) { - debug(1, L"Failed to fork %s!\n", fork_type); - job_mark_process_as_failed(j, p); - exec_error = true; - return false; - } - // This is the parent process. Store away information on the child, and - // possibly give it control over the terminal. - debug(2, L"Fork #%d, pid %d: %s for '%ls'", g_fork_count, pid, fork_type, - p->argv0()); - child_forked = true; - p->pid = pid; - set_child_group(j, p->pid); } + if (pid < 0) { + debug(1, L"Failed to fork %s!\n", fork_type); + job_mark_process_as_failed(j, p); + exec_error = true; + return false; + } + + // This is the parent process. Store away information on the child, and + // possibly give it control over the terminal. + debug(2, L"Fork #%d, pid %d: %s for '%ls'", g_fork_count, pid, fork_type, p->argv0()); + child_forked = true; + + p->pid = pid; + on_process_created(j, p->pid); + set_child_group(j, p->pid); + maybe_assign_terminal(j); + return true; }; @@ -1054,9 +1074,14 @@ void exec_job(parser_t &parser, job_t *j) { if (pid == 0) { job_mark_process_as_failed(j, p); exec_error = true; - } else { - child_spawned = true; + break; } + + // these are all things do_fork() takes care of normally: + p->pid = pid; + child_spawned = true; + on_process_created(j, p->pid); + maybe_assign_terminal(j); } else #endif { @@ -1066,10 +1091,6 @@ void exec_job(parser_t &parser, job_t *j) { } } - // This is the parent process. Store away information on the child, and possibly - // give it control over the terminal. - p->pid = pid; - set_child_group(j, p->pid); break; } diff --git a/src/postfork.cpp b/src/postfork.cpp index 766681ff5..16d273d9e 100644 --- a/src/postfork.cpp +++ b/src/postfork.cpp @@ -105,24 +105,23 @@ bool child_set_group(job_t *j, process_t *p) { /// group in the case of JOB_CONTROL, and we can give the new process group control of the terminal /// if it's to run in the foreground. bool set_child_group(job_t *j, pid_t child_pid) { - bool retval = true; - if (j->get_flag(JOB_CONTROL)) { - // New jobs have the pgid set to -2 - if (j->pgid == -2) { - j->pgid = child_pid; - } // The parent sets the child's group. This incurs the well-known unavoidable race with the // child exiting, so ignore ESRCH and EPERM (in case the pid was recycled). if (setpgid(child_pid, j->pgid) < 0) { if (errno != ESRCH && errno != EPERM) { safe_perror("setpgid"); + return false; } } } else { j->pgid = getpgrp(); } + return true; +} + +bool maybe_assign_terminal(job_t *j) { if (j->get_flag(JOB_TERMINAL) && j->get_flag(JOB_FOREGROUND)) { //!OCLINT(early exit) if (tcgetpgrp(STDIN_FILENO) == j->pgid) { // We've already assigned the process group control of the terminal when the first @@ -133,11 +132,11 @@ bool set_child_group(job_t *j, pid_t child_pid) { debug(4, L"Process group %d already has control of terminal\n", j->pgid); } else { // No need to duplicate the code here, a function already exists that does just this. - retval = terminal_give_to_job(j, false /*new job, so not continuing*/); + return terminal_give_to_job(j, false /*new job, so not continuing*/); } } - return retval; + return true; } /// Set up a childs io redirections. Should only be called by setup_child_process(). Does the diff --git a/src/postfork.h b/src/postfork.h index a4119418b..e2c102c6a 100644 --- a/src/postfork.h +++ b/src/postfork.h @@ -20,6 +20,7 @@ class process_t; bool set_child_group(job_t *j, pid_t child_pid); // called by parent bool child_set_group(job_t *j, process_t *p); // called by child +bool maybe_assign_terminal(job_t *j); /// Initialize a new child process. This should be called right away after forking in the child /// process. If job control is enabled for this job, the process is put in the process group of the