From f7b051905e66abf3bbdd8c6f0f2ce75f2d0a73a2 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Fri, 28 Jul 2017 21:52:01 -0500 Subject: [PATCH] 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. --- src/exec.cpp | 29 ++++++++++++++++++----------- src/postfork.cpp | 10 +++++----- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/exec.cpp b/src/exec.cpp index f0e1f6af8..bed8bf27c 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -517,7 +517,8 @@ void exec_job(parser_t &parser, job_t *j) { //set to true if we end up forking for this process bool child_forked = 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 // 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) { // This is the child process. Write out the contents of the pipeline. 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 // 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. - if (pipes_to_next_command) { + if (block_child) { 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); } 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'", g_fork_count, pid, p->argv0()); 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); } 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 // stdout and stderr, and then exit. 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 // 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. - if (pipes_to_next_command) { + if (block_child) { kill(p->pid, SIGSTOP); } + setup_child_process(j, p, process_net_io_chain); do_builtin_io(outbuff, outbuff_len, errbuff, errbuff_len); 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, p->argv0()); 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); } p->pid = pid; @@ -1109,13 +1114,15 @@ void exec_job(parser_t &parser, job_t *j) { if (pid == 0) { // This is the child process. 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 // 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. - if (pipes_to_next_command) { + if (block_child) { kill(p->pid, SIGSTOP); } + setup_child_process(j, p, process_net_io_chain); safe_launch_process(p, actual_cmd, argv, envv); // safe_launch_process _never_ returns... @@ -1128,7 +1135,7 @@ void exec_job(parser_t &parser, job_t *j) { exec_error = 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); } } @@ -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) { //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. diff --git a/src/postfork.cpp b/src/postfork.cpp index ecfd910fe..cb1e29fd9 100644 --- a/src/postfork.cpp +++ b/src/postfork.cpp @@ -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 /// 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. +/// 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 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) { bool ok = true; - //p is zero when EXEC_INTERNAL - if (p) { - ok = child_set_group(j, p); - } - 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)); if (p != 0 && !ok) { + debug_safe(0, "p!=0 && !ok"); exit_without_destructors(1); } }