[v2] jobs - Do not block when waiting on SIGCHLD

Commit Message

Herbert Xu May 6, 2018, 4:24 p.m. UTC

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 <[e-mail]>

Patch

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) {