child_setup_process to accept new termowner directly

Soon we will have more complicated logic around whether to call tcsetpgrp.
Prepare to centralize the logic by passing in the new term owner pgrp,
instead of having child_setup_process perform the decision.
This commit is contained in:
ridiculousfish 2019-07-01 10:57:09 -07:00
parent 8282369f45
commit b1a1b617f1
3 changed files with 11 additions and 7 deletions

View file

@ -300,7 +300,7 @@ void internal_exec(env_stack_t &vars, job_t *j, const io_chain_t &all_ios) {
// commands in the pipeline will apply to exec. However, using exec in a pipeline doesn't // commands in the pipeline will apply to exec. However, using exec in a pipeline doesn't
// really make sense, so I'm not trying to fix it here. // really make sense, so I'm not trying to fix it here.
auto redirs = dup2_list_t::resolve_chain(all_ios); auto redirs = dup2_list_t::resolve_chain(all_ios);
if (redirs && !child_setup_process(nullptr, false, *redirs)) { if (redirs && !child_setup_process(INVALID_PID, false, *redirs)) {
// Decrement SHLVL as we're removing ourselves from the shell "stack". // Decrement SHLVL as we're removing ourselves from the shell "stack".
auto shlvl_var = vars.get(L"SHLVL", ENV_GLOBAL | ENV_EXPORT); auto shlvl_var = vars.get(L"SHLVL", ENV_GLOBAL | ENV_EXPORT);
wcstring shlvl_str = L"0"; wcstring shlvl_str = L"0";
@ -443,9 +443,11 @@ static bool fork_child_for_process(const std::shared_ptr<job_t> &job, process_t
if (pid == 0) { if (pid == 0) {
// This is the child process. Setup redirections, print correct output to // This is the child process. Setup redirections, print correct output to
// stdout and stderr, and then exit. // stdout and stderr, and then exit.
maybe_t<pid_t> new_termowner{};
p->pid = getpid(); p->pid = getpid();
child_set_group(job.get(), p); child_set_group(job.get(), p);
child_setup_process(job.get(), true, dup2s); child_setup_process(job->wants_terminal() && job->is_foreground() ? job->pgid : INVALID_PID,
true, dup2s);
child_action(); child_action();
DIE("Child process returned control to fork_child lambda!"); DIE("Child process returned control to fork_child lambda!");
} }

View file

@ -134,7 +134,7 @@ bool set_child_group(job_t *j, pid_t child_pid) {
return true; return true;
} }
int child_setup_process(const job_t *job, bool is_forked, const dup2_list_t &dup2s) { int child_setup_process(pid_t new_termowner, bool is_forked, const dup2_list_t &dup2s) {
// Note we are called in a forked child. // Note we are called in a forked child.
for (const auto &act : dup2s.get_actions()) { for (const auto &act : dup2s.get_actions()) {
int err = act.target < 0 ? close(act.src) : dup2(act.src, act.target); int err = act.target < 0 ? close(act.src) : dup2(act.src, act.target);
@ -146,7 +146,7 @@ int child_setup_process(const job_t *job, bool is_forked, const dup2_list_t &dup
return err; return err;
} }
} }
if (job != nullptr && job->wants_terminal() && job->is_foreground()) { if (new_termowner != INVALID_PID) {
// Assign the terminal within the child to avoid the well-known race between tcsetgrp() in // Assign the terminal within the child to avoid the well-known race between tcsetgrp() in
// the parent and the child executing. We are not interested in error handling here, except // the parent and the child executing. We are not interested in error handling here, except
// we try to avoid this for non-terminals; in particular pipelines often make non-terminal // we try to avoid this for non-terminals; in particular pipelines often make non-terminal
@ -155,7 +155,7 @@ int child_setup_process(const job_t *job, bool is_forked, const dup2_list_t &dup
// Ensure this doesn't send us to the background (see #5963) // Ensure this doesn't send us to the background (see #5963)
signal(SIGTTIN, SIG_IGN); signal(SIGTTIN, SIG_IGN);
signal(SIGTTOU, SIG_IGN); signal(SIGTTOU, SIG_IGN);
(void)tcsetpgrp(STDIN_FILENO, job->pgid); (void)tcsetpgrp(STDIN_FILENO, new_termowner);
} }
} }
// Set the handling for job control signals back to the default. // Set the handling for job control signals back to the default.

View file

@ -27,9 +27,11 @@ bool child_set_group(job_t *j, process_t *p); // called by child
/// inside the exec function, which blocks all signals), and all IO redirections and other file /// inside the exec function, which blocks all signals), and all IO redirections and other file
/// descriptor actions are performed. /// descriptor actions are performed.
/// ///
/// Assign the terminal to new_termowner unless it is INVALID_PID.
///
/// \return 0 on sucess, -1 on failiure. When this function returns, signals are always unblocked. /// \return 0 on sucess, -1 on failiure. When this function returns, signals are always unblocked.
/// On failiure, signal handlers, io redirections and process group of the process is undefined. /// On failure, signal handlers, io redirections and process group of the process is undefined.
int child_setup_process(const job_t *job, bool is_forked, const dup2_list_t &dup2s); int child_setup_process(pid_t new_termowner, bool is_forked, const dup2_list_t &dup2s);
/// Call fork(), optionally waiting until we are no longer multithreaded. If the forked child /// Call fork(), optionally waiting until we are no longer multithreaded. If the forked child
/// doesn't do anything that could allocate memory, take a lock, etc. (like call exec), then it's /// doesn't do anything that could allocate memory, take a lock, etc. (like call exec), then it's