Re: PATCH: job control debug

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Fri, 7 Sep 2018 10:18:52 +0100
Peter Stephenson <p.stephenson@xxxxxxxxxxx> wrote:
> What do you think of the following?  If we are in list_pipe_job land, aka
> Rimmerworld, and we're attached to the terminal, keep the current signal
> behaviour.

No, don't believe this.  We get to this code way too often for this to
be useful.  (Why?  Is list_pipe_job being set too often or this how it's
supposed to work?)

> diff --git a/Src/exec.c b/Src/exec.c
> index 09ee132..4861ae5 100644
> --- a/Src/exec.c
> +++ b/Src/exec.c
> @@ -1029,6 +1029,13 @@ entersubsh(int flags)
>  		setpgrp(0L, jobtab[list_pipe_job].gleader);
>  		if (!(flags & ESUB_ASYNC))
>  		    attachtty(jobtab[thisjob].gleader);
> +	    } else if (gettygrp() == GETPGRP()) {
> +		/*
> +		 * There are races where if the process is attached
> +		 * to the terminal blocking SIGTTOU causes errors.
> +		 * So just leaves signals alone.
> +		 */
> +		job_control_ok = 1;
>  	    }
>  	}
>  	else if (!jobtab[thisjob].gleader ||

I'm seeing the terminal being attached to the first forked process which
is the group leader for the list pipe job.  So at this point  (the if
just above the patch) we're checking and finding that process is still
there and we're in the right process group, so everything is OK.

But it isn't OK.

I even tried moving this check after the point where we set the signal,
to see if there was a race with that process exiting after the check.
But that drew a blank, too --- it still thinks that process is there and
attached to the terminal and we're in the same process group.

But in that case we won't get SIGTTOU.  I can't see any further
setpgrp() that would put us in a different one, and I can't see any
attachtty() that would reassign the terminal.

Once we're in the process group and attached to the terminal, it
shouldn't matter if the other process exits.  This only gets confusing
if the shell has to decide who has grabbed the terminal from outside,
which shouldn't be a problem here --- it's just letting the 
synchronous processed manage themselves.

So I'm really confused.

Debug: I have pushed the job_control_debug branch.  Problems I was
seeing before were (i) because I was using an inconsistent ZLE library
that didn't instrument attachtty() (ii) because I wasn't flushing stderr
so I was seeing multiple occurrences of the same debug output.

pws