diff options
| -rw-r--r-- | ChangeLog | 4 | ||||
| -rw-r--r-- | Src/exec.c | 44 | ||||
| -rw-r--r-- | Src/jobs.c | 3 | ||||
| -rw-r--r-- | Src/signals.c | 16 |
4 files changed, 62 insertions, 5 deletions
diff --git a/ChangeLog b/ChangeLog index 29796ed01..add2ad6a5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2016-09-16 Peter Stephenson <p.stephenson@samsung.com> + * 39359: Src/exec.c, Src/jobs.c, Src/signals.c: Further fix on + top of 39331 for remaining race. Ensure process group of forked + superjob is sane. + * 39331: Src/exec.c, Src/jobs.c, Src/zsh.h: Partially fix problem occurring when a subjop in the RHS of a pipeline needs to be picked up by a forked zsh after ^Z when the original superjob diff --git a/Src/exec.c b/Src/exec.c index 21270c82d..0755d55cd 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -1652,7 +1652,13 @@ execpline(Estate state, wordcode slcode, int how, int last1) int synch[2]; struct timeval bgtime; + /* + * A pipeline with the shell handling the right + * hand side was stopped. We'll fork to allow + * it to continue. + */ if (pipe(synch) < 0 || (pid = zfork(&bgtime)) == -1) { + /* Failure */ if (pid < 0) { close(synch[0]); close(synch[1]); @@ -1666,6 +1672,18 @@ execpline(Estate state, wordcode slcode, int how, int last1) thisjob = newjob; } else if (pid) { + /* + * Parent: job control is here. If the job + * started for the RHS of the pipeline is still + * around, then its a SUBJOB and the job for + * earlier parts of the pipeeline is its SUPERJOB. + * The newly forked shell isn't recorded as a + * separate job here, just as list_pipe_pid. + * If the superjob exits (it may already have + * done so, see child branch below), we'll use + * list_pipe_pid to form the basis of a + * replacement job --- see SUBLEADER code above. + */ char dummy; lpforked = @@ -1692,9 +1710,33 @@ execpline(Estate state, wordcode slcode, int how, int last1) break; } else { + Job pjn = jobtab + list_pipe_job; close(synch[0]); entersubsh(ESUB_ASYNC); - if (jobtab[list_pipe_job].procs) { + /* + * At this point, we used to attach this process + * to the process group of list_pipe_job (the + * new superjob) any time that was still available. + * That caused problems when the fork happened + * early enough that the subjob is in that + * process group, since then we will stop this + * job when we signal the subjob, and we have + * no way here to know that we shouldn't also + * send STOP to itself, resulting in two stops. + * So don't do that if the original + * list_pipe_job has exited. + * + * The choice here needs to match the assumption + * made when handling SUBLEADER above that the + * process group is our own PID. I'm not sure + * if there's a potential race for that. But + * setting up a new process group if the + * superjob is still functioning seems the wrong + * thing to do. + */ + if (pjn->procs && + (pjn->stat & STAT_INUSE) && + !(pjn->stat & STAT_DONE)) { if (setpgrp(0L, mypgrp = jobtab[list_pipe_job].gleader) == -1) { setpgrp(0L, mypgrp = getpid()); diff --git a/Src/jobs.c b/Src/jobs.c index 9284c7124..d1b98ac4d 100644 --- a/Src/jobs.c +++ b/Src/jobs.c @@ -232,11 +232,12 @@ super_job(int sub) static int handle_sub(int job, int fg) { + /* job: superjob; sj: subjob. */ Job jn = jobtab + job, sj = jobtab + jn->other; if ((sj->stat & STAT_DONE) || (!sj->procs && !sj->auxprocs)) { struct process *p; - + for (p = sj->procs; p; p = p->next) { if (WIFSIGNALED(p->status)) { if (jn->gleader != mypgrp && jn->procs->next) diff --git a/Src/signals.c b/Src/signals.c index 2eefc07de..30dde713f 100644 --- a/Src/signals.c +++ b/Src/signals.c @@ -723,7 +723,7 @@ killjb(Job jn, int sig) { Process pn; int err = 0; - + if (jobbing) { if (jn->stat & STAT_SUPERJOB) { if (sig == SIGCONT) { @@ -731,11 +731,21 @@ killjb(Job jn, int sig) if (killpg(pn->pid, sig) == -1) if (kill(pn->pid, sig) == -1 && errno != ESRCH) err = -1; - + + /* + * Note this does not kill the last process, + * which is assumed to be the one controlling the + * subjob, i.e. the forked zsh that was originally + * list_pipe_pid... + */ for (pn = jn->procs; pn->next; pn = pn->next) if (kill(pn->pid, sig) == -1 && errno != ESRCH) err = -1; + /* + * ...we only continue that once the external processes + * currently associated with the subjob are finished. + */ if (!jobtab[jn->other].procs && pn) if (kill(pn->pid, sig) == -1 && errno != ESRCH) err = -1; @@ -744,7 +754,7 @@ killjb(Job jn, int sig) } if (killpg(jobtab[jn->other].gleader, sig) == -1 && errno != ESRCH) err = -1; - + if (killpg(jn->gleader, sig) == -1 && errno != ESRCH) err = -1; |