From d81af7216b3305a1aac211dc847dd1c191f3b307 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Tue, 18 Feb 2020 14:28:30 +0100 Subject: ash: eval: Reap zombies after built-in commands and functions Upstream commit: Date: Mon, 26 Mar 2018 23:55:50 +0800 eval: Reap zombies after built-in commands and functions Currently dash does not reap dead children after built-in commands or functions. This means that if you construct a loop consisting of solely built-in commands and functions, then zombies can hang around indefinitely. This patch fixes this by reaping when necessary after each built-in command and function. Reported-by: Denys Vlasenko Signed-off-by: Herbert Xu Signed-off-by: Denys Vlasenko --- shell/ash.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index 389db3cd0..8047cf98f 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -5355,10 +5355,10 @@ waitforjob(struct job *jp) { int st; - TRACE(("waitforjob(%%%d) called\n", jobno(jp))); + TRACE(("waitforjob(%%%d) called\n", jp ? jobno(jp) : 0)); INT_OFF; - while (jp->state == JOBRUNNING) { + while ((jp && jp->state == JOBRUNNING) || got_sigchld) { /* In non-interactive shells, we _can_ get * a keyboard signal here and be EINTRed, * but we just loop back, waiting for command to complete. @@ -5393,6 +5393,8 @@ waitforjob(struct job *jp) } INT_ON; + if (!jp) + return exitstatus; st = getstatus(jp); #if JOBS if (jp->jobctl) { @@ -10311,6 +10313,8 @@ evalcommand(union node *cmd, int flags) goto out; } + jp = NULL; + /* Execute the command. */ switch (cmdentry.cmdtype) { default: { @@ -10365,7 +10369,6 @@ evalcommand(union node *cmd, int flags) jp = makejob(/*cmd,*/ 1); if (forkshell(jp, cmd, FORK_FG) != 0) { /* parent */ - status = waitforjob(jp); INT_ON; TRACE(("forked child exited with %d\n", status)); break; @@ -10384,33 +10387,24 @@ evalcommand(union node *cmd, int flags) if (cmd_is_exec && argc > 1) listsetvar(varlist.list, VEXPORT); } - - /* Tight loop with builtins only: - * "while kill -0 $child; do true; done" - * will never exit even if $child died, unless we do this - * to reap the zombie and make kill detect that it's gone: */ - dowait(DOWAIT_NONBLOCK, NULL); - if (evalbltin(cmdentry.u.cmd, argc, argv, flags)) { if (exception_type == EXERROR && spclbltin <= 0) { FORCE_INT_ON; - goto readstatus; + break; } raise: longjmp(exception_handler->loc, 1); } - goto readstatus; + break; case CMDFUNCTION: - /* See above for the rationale */ - dowait(DOWAIT_NONBLOCK, NULL); if (evalfun(cmdentry.u.func, argc, argv, flags)) goto raise; - readstatus: - status = exitstatus; break; } /* switch */ + status = waitforjob(jp); + out: if (cmd->ncmd.redirect) popredir(/*drop:*/ cmd_is_exec); -- cgit v1.2.3-55-g6feb From 97edfc42f112a15828aaec886ef7012d24f34d5e Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Tue, 18 Feb 2020 14:37:56 +0100 Subject: ash: jobs - Do not block when waiting on SIGCHLD Upstream comment: Date: Mon, 7 May 2018 00:40:34 +0800 jobs - Do not block when waiting on SIGCHLD Because of the nature of SIGCHLD, the process may have already been waited on and therefore we must be prepared for the case that wait may block. So ensure that it doesn't by using WNOHANG. Furthermore, multiple jobs may have exited when gotsigchld is set. Therefore we need to wait until there are no zombies left. Lastly, waitforjob needs to be called with interrupts off and the original patch broke that. Fixes: 03876c0743a5 ("eval: Reap zombies after built-in...") Signed-off-by: Herbert Xu While at it, removed INT_ON/OFF in waitforjob() - it must be called from INT_OFF region anyway. Signed-off-by: Denys Vlasenko --- shell/ash.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index 8047cf98f..2b1378694 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -5357,8 +5357,16 @@ waitforjob(struct job *jp) TRACE(("waitforjob(%%%d) called\n", jp ? jobno(jp) : 0)); - INT_OFF; - while ((jp && jp->state == JOBRUNNING) || got_sigchld) { + if (!jp) { + int pid = got_sigchld; + + while (pid > 0) + pid = dowait(DOWAIT_NONBLOCK, NULL); + + return exitstatus; + } + + while (jp->state == JOBRUNNING) { /* In non-interactive shells, we _can_ get * a keyboard signal here and be EINTRed, * but we just loop back, waiting for command to complete. @@ -5391,10 +5399,7 @@ waitforjob(struct job *jp) */ dowait(DOWAIT_BLOCK, jp); } - INT_ON; - if (!jp) - return exitstatus; st = getstatus(jp); #if JOBS if (jp->jobctl) { @@ -10369,7 +10374,6 @@ evalcommand(union node *cmd, int flags) jp = makejob(/*cmd,*/ 1); if (forkshell(jp, cmd, FORK_FG) != 0) { /* parent */ - INT_ON; TRACE(("forked child exited with %d\n", status)); break; } @@ -10387,11 +10391,9 @@ evalcommand(union node *cmd, int flags) if (cmd_is_exec && argc > 1) listsetvar(varlist.list, VEXPORT); } - if (evalbltin(cmdentry.u.cmd, argc, argv, flags)) { - if (exception_type == EXERROR && spclbltin <= 0) { - FORCE_INT_ON; - break; - } + if (evalbltin(cmdentry.u.cmd, argc, argv, flags) + && !(exception_type == EXERROR && spclbltin <= 0) + ) { raise: longjmp(exception_handler->loc, 1); } @@ -10404,6 +10406,7 @@ evalcommand(union node *cmd, int flags) } /* switch */ status = waitforjob(jp); + FORCE_INT_ON; out: if (cmd->ncmd.redirect) -- cgit v1.2.3-55-g6feb From 47eb979404735b9528538968cb5eaac7355a0c5a Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Tue, 18 Feb 2020 15:37:43 +0100 Subject: ash: jobs: Only clear gotsigchld when waiting for everything Upstream commit: Date: Sat, 19 May 2018 02:39:41 +0800 jobs: Only clear gotsigchld when waiting for everything The gotsigchld flag is always cleared in dowait but not all callers of dowait will wait for everything. In particular, when jp is set we only wait until the set job isn't running anymore. This patch fixes this by only clearing gotsigchld if jp is unset. It also changes the waitcmd to actually set jp which corresponds to the behaviour of bash/ksh93/mksh. The only other caller of dowait that doesn't wait for everything is the jobless reaper. This is in fact redundant now that we wait after every simple command. This patch removes it. Finally as every caller of dowait needs to wait until either the given job is not running, or until all terminated jobs have been processed, this patch moves the loop into dowait itself. Fixes: 03876c0743a5 ("eval: Reap zombies after built-in...") Signed-off-by: Herbert Xu Signed-off-by: Denys Vlasenko --- shell/ash.c | 122 ++++++++++++++++++++++++++++-------------------------------- 1 file changed, 56 insertions(+), 66 deletions(-) (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index 2b1378694..ff594050c 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -3810,8 +3810,6 @@ static struct job *jobtab; //5 static unsigned njobs; //4 /* current job */ static struct job *curjob; //lots -/* number of presumed living untracked jobs */ -static int jobless; //4 #if 0 /* Bash has a feature: it restores termios after a successful wait for @@ -4331,7 +4329,7 @@ wait_block_or_sig(int *status) #endif static int -dowait(int block, struct job *job) +waitone(int block, struct job *job) { int pid; int status; @@ -4432,10 +4430,6 @@ dowait(int block, struct job *job) goto out; } /* The process wasn't found in job list */ -#if JOBS - if (!WIFSTOPPED(status)) - jobless--; -#endif out: INT_ON; @@ -4460,6 +4454,20 @@ dowait(int block, struct job *job) return pid; } +static int +dowait(int block, struct job *jp) +{ + int pid = block == DOWAIT_NONBLOCK ? got_sigchld : 1; + + while (jp ? jp->state == JOBRUNNING : pid > 0) { + if (!jp) + got_sigchld = 0; + pid = waitone(block, jp); + } + + return pid; +} + #if JOBS static void showjob(struct job *jp, int mode) @@ -4548,8 +4556,7 @@ showjobs(int mode) TRACE(("showjobs(0x%x) called\n", mode)); /* Handle all finished jobs */ - while (dowait(DOWAIT_NONBLOCK, NULL) > 0) - continue; + dowait(DOWAIT_NONBLOCK, NULL); for (jp = curjob; jp; jp = jp->prev_job) { if (!(mode & SHOW_CHANGED) || jp->changed) { @@ -4666,10 +4673,10 @@ waitcmd(int argc UNUSED_PARAM, char **argv) #else dowait(DOWAIT_BLOCK_OR_SIG, NULL); #endif - /* if child sends us a signal *and immediately exits*, - * dowait() returns pid > 0. Check this case, - * not "if (dowait() < 0)"! - */ + /* if child sends us a signal *and immediately exits*, + * dowait() returns pid > 0. Check this case, + * not "if (dowait() < 0)"! + */ if (pending_sig) goto sigout; #if BASH_WAIT_N @@ -4705,11 +4712,9 @@ waitcmd(int argc UNUSED_PARAM, char **argv) job = getjob(*argv, 0); } /* loop until process terminated or stopped */ - while (job->state == JOBRUNNING) { - dowait(DOWAIT_BLOCK_OR_SIG, NULL); - if (pending_sig) - goto sigout; - } + dowait(DOWAIT_BLOCK_OR_SIG, NULL); + if (pending_sig) + goto sigout; job->waited = 1; retval = getstatus(job); repeat: ; @@ -5261,7 +5266,6 @@ forkchild(struct job *jp, union node *n, int mode) #endif for (jp = curjob; jp; jp = jp->prev_job) freejob(jp); - jobless = 0; } /* Called after fork(), in parent */ @@ -5272,13 +5276,8 @@ static void forkparent(struct job *jp, union node *n, int mode, pid_t pid) { TRACE(("In parent shell: child = %d\n", pid)); - if (!jp) { - /* jp is NULL when called by openhere() for heredoc support */ - while (jobless && dowait(DOWAIT_NONBLOCK, NULL) > 0) - continue; - jobless++; + if (!jp) /* jp is NULL when called by openhere() for heredoc support */ return; - } #if JOBS if (mode != FORK_NOJOB && jp->jobctl) { int pgrp; @@ -5357,48 +5356,39 @@ waitforjob(struct job *jp) TRACE(("waitforjob(%%%d) called\n", jp ? jobno(jp) : 0)); - if (!jp) { - int pid = got_sigchld; - - while (pid > 0) - pid = dowait(DOWAIT_NONBLOCK, NULL); - + /* In non-interactive shells, we _can_ get + * a keyboard signal here and be EINTRed, but we just loop + * inside dowait(), waiting for command to complete. + * + * man bash: + * "If bash is waiting for a command to complete and receives + * a signal for which a trap has been set, the trap + * will not be executed until the command completes." + * + * Reality is that even if trap is not set, bash + * will not act on the signal until command completes. + * Try this. sleep5intoff.c: + * #include + * #include + * int main() { + * sigset_t set; + * sigemptyset(&set); + * sigaddset(&set, SIGINT); + * sigaddset(&set, SIGQUIT); + * sigprocmask(SIG_BLOCK, &set, NULL); + * sleep(5); + * return 0; + * } + * $ bash -c './sleep5intoff; echo hi' + * ^C^C^C^C <--- pressing ^C once a second + * $ _ + * $ bash -c './sleep5intoff; echo hi' + * ^\^\^\^\hi <--- pressing ^\ (SIGQUIT) + * $ _ + */ + dowait(jp ? DOWAIT_BLOCK : DOWAIT_NONBLOCK, jp); + if (!jp) return exitstatus; - } - - while (jp->state == JOBRUNNING) { - /* In non-interactive shells, we _can_ get - * a keyboard signal here and be EINTRed, - * but we just loop back, waiting for command to complete. - * - * man bash: - * "If bash is waiting for a command to complete and receives - * a signal for which a trap has been set, the trap - * will not be executed until the command completes." - * - * Reality is that even if trap is not set, bash - * will not act on the signal until command completes. - * Try this. sleep5intoff.c: - * #include - * #include - * int main() { - * sigset_t set; - * sigemptyset(&set); - * sigaddset(&set, SIGINT); - * sigaddset(&set, SIGQUIT); - * sigprocmask(SIG_BLOCK, &set, NULL); - * sleep(5); - * return 0; - * } - * $ bash -c './sleep5intoff; echo hi' - * ^C^C^C^C <--- pressing ^C once a second - * $ _ - * $ bash -c './sleep5intoff; echo hi' - * ^\^\^\^\hi <--- pressing ^\ (SIGQUIT) - * $ _ - */ - dowait(DOWAIT_BLOCK, jp); - } st = getstatus(jp); #if JOBS -- cgit v1.2.3-55-g6feb From 23bc562a0556d1c0ddad4252fa8d46c863b5fa88 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Tue, 18 Feb 2020 16:46:01 +0100 Subject: ash,hush: add comment about masked SIGCHLD, handle SIG_IGNed SIGHUP as in bash Signed-off-by: Denys Vlasenko --- shell/ash.c | 26 ++++++++++++++++++++------ shell/hush.c | 20 +++++++++++++++++--- 2 files changed, 37 insertions(+), 9 deletions(-) (limited to 'shell') diff --git a/shell/ash.c b/shell/ash.c index ff594050c..fea4b10a7 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -4307,8 +4307,19 @@ wait_block_or_sig(int *status) #if 1 sigfillset(&mask); sigprocmask2(SIG_SETMASK, &mask); /* mask is updated */ - while (!got_sigchld && !pending_sig) + while (!got_sigchld && !pending_sig) { sigsuspend(&mask); + /* ^^^ add "sigdelset(&mask, SIGCHLD);" before sigsuspend + * to make sure SIGCHLD is not masked off? + * It was reported that this: + * fn() { : | return; } + * shopt -s lastpipe + * fn + * exec ash SCRIPT + * under bash 4.4.23 runs SCRIPT with SIGCHLD masked, + * making "wait" commands in SCRIPT block forever. + */ + } sigprocmask(SIG_SETMASK, &mask, NULL); #else /* unsafe: a signal can set pending_sig after check, but before pause() */ while (!got_sigchld && !pending_sig) @@ -14170,11 +14181,6 @@ init(void) sigmode[SIGCHLD - 1] = S_DFL; /* ensure we install handler even if it is SIG_IGNed */ setsignal(SIGCHLD); - /* bash re-enables SIGHUP which is SIG_IGNed on entry. - * Try: "trap '' HUP; bash; echo RET" and type "kill -HUP $$" - */ - signal(SIGHUP, SIG_DFL); - { char **envp; const char *p; @@ -14512,6 +14518,14 @@ int ash_main(int argc UNUSED_PARAM, char **argv) } #endif state4: /* XXX ??? - why isn't this before the "if" statement */ + + /* Interactive bash re-enables SIGHUP which is SIG_IGNed on entry. + * Try: + * trap '' hup; bash; echo RET # type "kill -hup $$", see SIGHUP having effect + * trap '' hup; bash -c 'kill -hup $$; echo ALIVE' # here SIGHUP is SIG_IGNed + */ + signal(SIGHUP, SIG_DFL); + cmdloop(1); } #if PROFILE diff --git a/shell/hush.c b/shell/hush.c index 6e44d4e11..bced388bf 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -9775,10 +9775,14 @@ static void install_sighandlers(unsigned mask) */ if (sig == SIGCHLD) continue; - /* bash re-enables SIGHUP which is SIG_IGNed on entry. - * Try: "trap '' HUP; bash; echo RET" and type "kill -HUP $$" + /* Interactive bash re-enables SIGHUP which is SIG_IGNed on entry. + * Try: + * trap '' hup; bash; echo RET # type "kill -hup $$", see SIGHUP having effect + * trap '' hup; bash -c 'kill -hup $$; echo ALIVE' # here SIGHUP is SIG_IGNed */ - //if (sig == SIGHUP) continue; - TODO? + if (sig == SIGHUP && G_interactive_fd) + continue; + /* Unless one of the above signals, is it SIG_IGN? */ if (old_handler == SIG_IGN) { /* oops... restore back to IGN, and record this fact */ install_sighandler(sig, old_handler); @@ -11554,6 +11558,16 @@ static int wait_for_child_or_signal(struct pipe *waitfor_pipe, pid_t waitfor_pid /* It is vitally important for sigsuspend that SIGCHLD has non-DFL handler! */ /* Note: sigsuspend invokes signal handler */ sigsuspend(&oldset); + /* ^^^ add "sigdelset(&oldset, SIGCHLD)" before sigsuspend + * to make sure SIGCHLD is not masked off? + * It was reported that this: + * fn() { : | return; } + * shopt -s lastpipe + * fn + * exec hush SCRIPT + * under bash 4.4.23 runs SCRIPT with SIGCHLD masked, + * making "wait" commands in SCRIPT block forever. + */ restore: sigprocmask(SIG_SETMASK, &oldset, NULL); check_sig: -- cgit v1.2.3-55-g6feb