Split child_set_group from setup_child_process

setup_child_process blocks in the case of IO_FILE, meaning it can't
be called before child processes SIGSTOP.
This commit is contained in:
Mahmoud Al-Qudsi 2017-07-28 21:52:01 -05:00 committed by Kurtis Rader
parent 8537cc982e
commit f7b051905e
2 changed files with 23 additions and 16 deletions

View File

@ -517,7 +517,8 @@ void exec_job(parser_t &parser, job_t *j) {
//set to true if we end up forking for this process //set to true if we end up forking for this process
bool child_forked = false; bool child_forked = false;
bool child_spawned = false; bool child_spawned = false;
// bool child_blocked = false; // bool block_child = !needs_keepalive;
bool block_child = true;
// The pipes the current process write to and read from. Unfortunately these can't be just // The pipes the current process write to and read from. Unfortunately these can't be just
// allocated on the stack, since j->io wants shared_ptr. // allocated on the stack, since j->io wants shared_ptr.
@ -883,13 +884,15 @@ void exec_job(parser_t &parser, job_t *j) {
if (pid == 0) { if (pid == 0) {
// This is the child process. Write out the contents of the pipeline. // This is the child process. Write out the contents of the pipeline.
p->pid = getpid(); p->pid = getpid();
setup_child_process(j, p, process_net_io_chain); blocked_pid = -1;
child_set_group(j, p);
// Make child processes pause after executing setup_child_process() to give down-chain // Make child processes pause after executing setup_child_process() to give down-chain
// commands in the job a chance to join their process group and read their pipes. // commands in the job a chance to join their process group and read their pipes.
// The process will be resumed when the next command in the chain is started. // The process will be resumed when the next command in the chain is started.
if (pipes_to_next_command) { if (block_child) {
kill(p->pid, SIGSTOP); kill(p->pid, SIGSTOP);
} }
setup_child_process(j, p, process_net_io_chain);
exec_write_and_exit(block_output_io_buffer->fd, buffer, count, status); exec_write_and_exit(block_output_io_buffer->fd, buffer, count, status);
} else { } else {
@ -898,7 +901,7 @@ void exec_job(parser_t &parser, job_t *j) {
debug(2, L"Fork #%d, pid %d: internal block or function for '%ls'", debug(2, L"Fork #%d, pid %d: internal block or function for '%ls'",
g_fork_count, pid, p->argv0()); g_fork_count, pid, p->argv0());
child_forked = true; child_forked = true;
if (pipes_to_next_command) { if (block_child) {
debug(2, L"Blocking process %d waiting for next command in chain.\n", pid); debug(2, L"Blocking process %d waiting for next command in chain.\n", pid);
} }
p->pid = pid; p->pid = pid;
@ -1014,13 +1017,15 @@ void exec_job(parser_t &parser, job_t *j) {
// 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.
p->pid = getpid(); p->pid = getpid();
setup_child_process(j, p, process_net_io_chain); blocked_pid = -1;
child_set_group(j, p);
// Make child processes pause after executing setup_child_process() to give down-chain // Make child processes pause after executing setup_child_process() to give down-chain
// commands in the job a chance to join their process group and read their pipes. // commands in the job a chance to join their process group and read their pipes.
// The process will be resumed when the next command in the chain is started. // The process will be resumed when the next command in the chain is started.
if (pipes_to_next_command) { if (block_child) {
kill(p->pid, SIGSTOP); kill(p->pid, SIGSTOP);
} }
setup_child_process(j, p, process_net_io_chain);
do_builtin_io(outbuff, outbuff_len, errbuff, errbuff_len); do_builtin_io(outbuff, outbuff_len, errbuff, errbuff_len);
exit_without_destructors(p->status); exit_without_destructors(p->status);
@ -1030,7 +1035,7 @@ void exec_job(parser_t &parser, job_t *j) {
debug(2, L"Fork #%d, pid %d: internal builtin for '%ls'", g_fork_count, pid, debug(2, L"Fork #%d, pid %d: internal builtin for '%ls'", g_fork_count, pid,
p->argv0()); p->argv0());
child_forked = true; child_forked = true;
if (pipes_to_next_command) { if (block_child) {
debug(2, L"Blocking process %d waiting for next command in chain.\n", pid); debug(2, L"Blocking process %d waiting for next command in chain.\n", pid);
} }
p->pid = pid; p->pid = pid;
@ -1109,13 +1114,15 @@ void exec_job(parser_t &parser, job_t *j) {
if (pid == 0) { if (pid == 0) {
// This is the child process. // This is the child process.
p->pid = getpid(); p->pid = getpid();
setup_child_process(j, p, process_net_io_chain); blocked_pid = -1;
child_set_group(j, p);
// Make child processes pause after executing setup_child_process() to give down-chain // Make child processes pause after executing setup_child_process() to give down-chain
// commands in the job a chance to join their process group and read their pipes. // commands in the job a chance to join their process group and read their pipes.
// The process will be resumed when the next command in the chain is started. // The process will be resumed when the next command in the chain is started.
if (pipes_to_next_command) { if (block_child) {
kill(p->pid, SIGSTOP); kill(p->pid, SIGSTOP);
} }
setup_child_process(j, p, process_net_io_chain);
safe_launch_process(p, actual_cmd, argv, envv); safe_launch_process(p, actual_cmd, argv, envv);
// safe_launch_process _never_ returns... // safe_launch_process _never_ returns...
@ -1128,7 +1135,7 @@ void exec_job(parser_t &parser, job_t *j) {
exec_error = true; exec_error = true;
} }
child_forked = true; child_forked = true;
if (pipes_to_next_command) { if (block_child) {
debug(2, L"Blocking process %d waiting for next command in chain.\n", pid); debug(2, L"Blocking process %d waiting for next command in chain.\n", pid);
} }
} }
@ -1147,7 +1154,7 @@ void exec_job(parser_t &parser, job_t *j) {
} }
} }
bool child_blocked = child_forked && pipes_to_next_command; //to make things clearer below bool child_blocked = block_child && child_forked;
if (child_blocked) { if (child_blocked) {
//we have to wait to ensure the child has set their progress group and is in SIGSTOP state //we have to wait to ensure the child has set their progress group and is in SIGSTOP state
//otherwise, we can later call SIGCONT before they call SIGSTOP and they'll be blocked indefinitely. //otherwise, we can later call SIGCONT before they call SIGSTOP and they'll be blocked indefinitely.

View File

@ -125,6 +125,9 @@ bool child_set_group(job_t *j, process_t *p) {
/// and we can give the new process group control of the terminal if it's to run in the foreground. Note that /// and we can give the new process group control of the terminal if it's to run in the foreground. Note that
/// we can guarantee the child won't try to read from the terminal before we've had a chance to run this code, /// we can guarantee the child won't try to read from the terminal before we've had a chance to run this code,
/// because we haven't woken them up with a SIGCONT yet. /// because we haven't woken them up with a SIGCONT yet.
/// This musn't be called as a part of setup_child_process because that can hang indefinitely until data is
/// available to read/write in the case of IO_FILE, which means we'll never reach our SIGSTOP and everything
/// hangs.
bool set_child_group(job_t *j, pid_t child_pid) { bool set_child_group(job_t *j, pid_t child_pid) {
bool retval = true; bool retval = true;
@ -259,14 +262,11 @@ static int handle_child_io(const io_chain_t &io_chain) {
int setup_child_process(job_t *j, process_t *p, const io_chain_t &io_chain) { int setup_child_process(job_t *j, process_t *p, const io_chain_t &io_chain) {
bool ok = true; bool ok = true;
//p is zero when EXEC_INTERNAL
if (p) {
ok = child_set_group(j, p);
}
if (ok) { if (ok) {
//In the case of IO_FILE, this can hang until data is available to read/write!
ok = (0 == handle_child_io(io_chain)); ok = (0 == handle_child_io(io_chain));
if (p != 0 && !ok) { if (p != 0 && !ok) {
debug_safe(0, "p!=0 && !ok");
exit_without_destructors(1); exit_without_destructors(1);
} }
} }