Re: Bug related to stdin/always/jobcontrol

[Peter Stephenson <p.stephenson@samsung.com>]

On Mon, 5 Sep 2016 16:42:07 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> > juno% v() { { vim - } always { true } }
> > juno% ls | v
> > ^Z
> > zsh: running    v
> > juno% fg
> > fg: no current job
> 
> It looks like the job state is quite seriously challenged.

I think I might be getting close with the attached patch, but not quite
far enough so I know I'm not doing something fundamentally screwy.
With it, I get as far as continuing the process, but vim gets stopped
again because it can't write to the terminal.  Hence I'm suspecting
something's still amiss in process group handling.  I never understood
process group handling.  In case anyone does, as you'll see if you try
it out, you get the vim process and the fix-up subprocess zsh (as below)
in a process group which is the PID of an exited process.  So first
guess is we need to create a new process group, or something.

That exited process is what was originally SUPERJOB, which is the ls which
has exited (I think: I haven't trapped it in the act but this seems to
be the only thing that makes sense).  The newly forked zsh is the result
of the ^Z and so needs to be in charge of the vim process (and, in the
zsh model, subjob) --- again, I think, because that's basically why we
do the fork, so we can still bring the right hand side of the pipeline
which was originally in the current shell into the foreground.

> When fg is run, the state of the job you can see has flags 0x2012:
> 
> #define STAT_STOPPED	(0x0002) /* all procs stopped or exited          */
> #define STAT_LOCKED	(0x0010) /* shell is finished creating this job, */
>                                  /*   may be deleted from job table      */
> #define STAT_SUBLEADER  (0x2000) /* is super-job, but leader is sub-shell */
> 
> This doesn't have STAT_INUSE, so it's not clear "jobs" should be
> reporting it at all.  Adding STAT_INUSE and fg'ing doesn't help; it does
> attempt to bring the command to the foreground but then gets stuck and
> stays stuck even if the vim command is killed from elsewhere.
> 
> pws      27402 25329  7 16:28 pts/1    00:00:01 ./zsh
> pws      27423 27402  0 16:29 pts/1    00:00:00 vim -
> pws      27425 27402  0 16:29 pts/1    00:00:00 ./zsh
> 
> That first ./zsh is the parent shell; I guess 27425 is the subshell
> process.

This appears to be the case.

I've added the INUSE back in, because the job doesn't seem to be any use
to person nor best without it.

I've then made it look for a SUBJOB of which it can become SUPERJOB, and
fixed up the associations.  This also assumes I've worked out the
difference between being a SUBLEADER and a SUPERJOB.  Note: I'm not
sure if we need to search for an existing SUPERJOB first because I'm
not sure in what circumstances we can get to the point we have.

The final hunk is an obvious typo --- "other" is always a job (possibly
sounds better in French).

Now fg is trying to do something looks plausible, but with the effect
noted above.

pws

diff --git a/Src/exec.c b/Src/exec.c
index cfd633a..2638b01 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1570,6 +1570,7 @@ execpline(Estate state, wordcode slcode, int how, int last1)
 
 	    if (nowait) {
 		if(!pline_level) {
+		    int jobsub;
 		    struct process *pn, *qn;
 
 		    curjob = newjob;
@@ -1593,7 +1594,23 @@ execpline(Estate state, wordcode slcode, int how, int last1)
 		    }
 
 		    jn->stat &= ~(STAT_DONE | STAT_NOPRINT);
-		    jn->stat |= STAT_STOPPED | STAT_CHANGED | STAT_LOCKED;
+		    jn->stat |= STAT_STOPPED | STAT_CHANGED | STAT_LOCKED |
+			STAT_INUSE;
+		    /*
+		     * Pick up any subjob that's still lying around
+		     * as it's now our responsibility.
+		     * If we find it we're a regular SUPERJOB
+		     * instead of a mere SUBLEADER.
+		     */
+		    for (jobsub = 1; jobsub <= maxjob; jobsub++) {
+			Job jnsub = jobtab + jobsub;
+			if (jnsub->stat & STAT_SUBJOB) {
+			    jn->other = jobsub;
+			    jn->stat |= STAT_SUPERJOB;
+			    jn->stat &= ~STAT_SUBLEADER;
+			    jnsub->other = newjob;
+			}
+		    }
 		    printjob(jn, !!isset(LONGLISTJOBS), 1);
 		}
 		else if (newjob != list_pipe_job)
@@ -1668,7 +1685,7 @@ execpline(Estate state, wordcode slcode, int how, int last1)
 			    jobtab[list_pipe_job].other = newjob;
 			    jobtab[list_pipe_job].stat |= STAT_SUPERJOB;
 			    jn->stat |= STAT_SUBJOB | STAT_NOPRINT;
-			    jn->other = pid;
+			    jn->other = list_pipe_job;
 			}
 			if ((list_pipe || last1) && hasprocs(list_pipe_job))
 			    killpg(jobtab[list_pipe_job].gleader, SIGSTOP);