On Sun, May 06, 2018 at 03:53:43PM +0200, Jilles Tjoelker wrote: > > Now each of the first four executable lines of waitforjob() does > something different for jp == NULL and jp != NULL. It probably makes > more sense to separate the jp == NULL case into a new function. Thanks, that's a good point. However, because it's convenient to call waitforjob with NULL in eval, I'll keep it in the same function but create two distinct code paths for it. However, a bigger issue is that the original fix was incomplete. In particular, multiple jobs may have exited when gotsigchld is set. But the loop will exit after just waiting for one job if no new jobs exit triggering a new SIGCHLD. ---8<--- 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. Fixes: 03876c0743a5 ("eval: Reap zombies after built-in...") Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> diff --git a/src/jobs.c b/src/jobs.c index 1a97c54..606d603 100644 --- a/src/jobs.c +++ b/src/jobs.c @@ -975,10 +975,17 @@ waitforjob(struct job *jp) int st; TRACE(("waitforjob(%%%d) called\n", jp ? jobno(jp) : 0)); - while ((jp && jp->state == JOBRUNNING) || gotsigchld) - dowait(DOWAIT_BLOCK, jp); - if (!jp) + if (!jp) { + int pid = gotsigchld; + + while (pid > 0) + pid = dowait(DOWAIT_NORMAL, NULL); + return exitstatus; + } + + while (jp->state == JOBRUNNING) + dowait(DOWAIT_BLOCK, jp); st = getstatus(jp); #if JOBS if (jp->jobctl) { -- Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe dash" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html