zsh-workers
 help / color / mirror / code / Atom feed
From: Peter Stephenson <p.w.stephenson@ntlworld.com>
To: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: Zsh: [7] + 23074 suspended (tty output)
Date: Sun, 16 Sep 2018 18:51:17 +0100	[thread overview]
Message-ID: <20180916185117.1c0eaba1@pws-HP.localdomain> (raw)
In-Reply-To: <9b969afc-6241-b7ab-32cc-cd3b37d6cb9c@xk2c.de>

On Sun, 16 Sep 2018 17:40:48 +0200
TS <debts@xk2c.de> wrote:
> I have now tried different iterations of that minimal .zshrc.
> It seems only when less is wrapped inside a function i get this
> "zsh: suspended (tty output)".

Thanks for that --- it does help to have all the reproduction detail inline
(or attached) in a post on the list.

> But when less is wrapped inside a function like this, it is rather reliable
> replicable.

All the previous forms of this bug show this when inside a sourced file, so e.g.
testfile containing

echo | { cat | more; }

followed by ". ./testfile" shows this up.

I've rebased job_control_debug2 on top of all the latest fixes to track
this down.  (Just the sort of thing I like doing in my hotel room on
holiday.)

The issue is that the list_pipe_job mechanism is being applied twice,
once for the "." and once for the nested code (a function in your case,
a nested pipeline in the above case).  Regular readers will know one
application is quite enough --- quite whether doing this twice is a
good idea is anybody's guess.  Anyway, the first job is marked as being
in control of the terminal; then the second job gets the terminal.  The
first job exits and the shell erroneously thinks it was still in
control of the terminal and swipes it back from under the feet of the
second.

We need to keep track of which job's process group last got the
terminal.  I think the combination of recording this in attachtty() and
also in the recently added code that fixes up the records in the main
shell from what the new subshell just did should do the trick --- we're
still protected by the STAT_NOSTTY check with the additional safety.
However, I'm not sure we want to rush this fix out to all and sundry.
The trouble is, clearly no one's testing things until they're released
(which is why the releases are broken).  If this is likely to come
unstuck anywhere, I'd suspect use of ^Z and fg which is applying
the most additional stress to the foreground terminal machinations,
so if you have things you do with ^Z and complex shell constructs
please try them out.

I've also tightened up in entersubsh() so that if the process is
asynchronous it doesn't pass up the status --- that's all the
exec.c hunks are doing.

diff --git a/Src/exec.c b/Src/exec.c
index b9af9ea63..a667b078d 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1036,7 +1036,7 @@ entersubsh(int flags, struct entersubsh_ret *retp)
 		if (!(flags & ESUB_ASYNC))
 		    attachtty(jobtab[thisjob].gleader);
 	    }
-	    if (retp) {
+	    if (retp && !(flags & ESUB_ASYNC)) {
 		retp->gleader = jobtab[list_pipe_job].gleader;
 		retp->list_pipe_job = list_pipe_job;
 	    }
@@ -1058,12 +1058,13 @@ entersubsh(int flags, struct entersubsh_ret *retp)
 		!jobtab[list_pipe_job].gleader)
 		jobtab[list_pipe_job].gleader = jobtab[thisjob].gleader;
 	    setpgrp(0L, jobtab[thisjob].gleader);
-	    if (!(flags & ESUB_ASYNC))
+	    if (!(flags & ESUB_ASYNC)) {
 		attachtty(jobtab[thisjob].gleader);
-	    if (retp) {
-		retp->gleader = jobtab[thisjob].gleader;
-		if (list_pipe_job != thisjob)
-		    retp->list_pipe_job = list_pipe_job;
+		if (retp) {
+		    retp->gleader = jobtab[thisjob].gleader;
+		    if (list_pipe_job != thisjob)
+			retp->list_pipe_job = list_pipe_job;
+		}
 	    }
 	}
     }
diff --git a/Src/jobs.c b/Src/jobs.c
index db2e87ec1..2d58319a8 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -40,6 +40,11 @@ mod_export pid_t origpgrp;
 
 /**/
 mod_export pid_t mypgrp;
+
+/* the last process group to attach to the terminal */
+
+/**/
+pid_t last_attached_pgrp;
  
 /* the job we are working on */
  
@@ -1405,6 +1410,11 @@ addproc(pid_t pid, char *text, int aux, struct timeval *bgtime,
 	    jobtab[thisjob].gleader = gleader;
 	    if (list_pipe_job_used != -1)
 		jobtab[list_pipe_job_used].gleader = gleader;
+	    /*
+	     * Record here this is the latest process group to grab the
+	     * terminal as attachtty() was run in the subshell.
+	     */
+	    last_attached_pgrp = gleader;
 	} else if (!jobtab[thisjob].gleader)
 		jobtab[thisjob].gleader = pid;
 	/* attach this process to end of process list of current job */
diff --git a/Src/signals.c b/Src/signals.c
index 99aad0fab..26d88abc2 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -540,8 +540,8 @@ wait_for_processes(void)
 		if (WIFEXITED(status) &&
 		    pn->pid == jn->gleader &&
 		    killpg(pn->pid, 0) == -1) {
-		    jn->gleader = 0;
-		    if (!(jn->stat & STAT_NOSTTY)) {
+		    if (last_attached_pgrp == jn->gleader &&
+			!(jn->stat & STAT_NOSTTY)) {
 			/*
 			 * This PID was in control of the terminal;
 			 * reclaim terminal now it has exited.
@@ -552,6 +552,7 @@ wait_for_processes(void)
 			attachtty(mypgrp);
 			adjustwinsize(0);
 		    }
+		    jn->gleader = 0;
 		}
 	    }
 	    update_job(jn);
diff --git a/Src/utils.c b/Src/utils.c
index 075d27241..5a9fbdd32 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -4670,6 +4670,10 @@ attachtty(pid_t pgrp)
 		ep = 1;
 	    }
 	}
+	else
+	{
+	    last_attached_pgrp = pgrp;
+	}
     }
 }
 

  reply	other threads:[~2018-09-16 18:03 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-15 17:22 TS
2018-09-15 18:10 ` Bart Schaefer
2018-09-15 18:51   ` TS
2018-09-15 19:06     ` TS
2018-09-16  8:40       ` Vincent Lefevre
2018-09-16 15:40         ` TS
2018-09-16 17:51           ` Peter Stephenson [this message]
2018-09-17 20:13           ` TS
2018-09-18 17:59             ` Peter Stephenson
2018-09-18  1:16         ` Vincent Lefevre
2018-09-18 17:58           ` Peter Stephenson
2018-09-20 12:01             ` Vincent Lefevre
2018-09-20 16:11               ` Daniel Shahaf
2018-09-18 19:30           ` TS
2018-09-19  3:38             ` TS
2018-09-19 14:23               ` Daniel Tameling
2018-09-19 16:12               ` Peter Stephenson
2018-09-19 18:15                 ` TS
2018-09-17 14:01 ` Sebastian Gniazdowski
2018-09-17 14:49   ` Valgrind tests (was: Re: Zsh: [7] + 23074 suspended (tty output)) Daniel Shahaf
2018-09-18  5:21     ` Sebastian Gniazdowski
2018-09-18 15:55       ` Daniel Shahaf
2018-09-19  5:04         ` Sebastian Gniazdowski
2019-07-03 23:29         ` Sebastian Gniazdowski
2019-07-03 23:31           ` Sebastian Gniazdowski
2019-07-03 23:45             ` Sebastian Gniazdowski
2019-07-13 14:31               ` Sebastian Gniazdowski
2019-07-14  6:39                 ` Daniel Shahaf
2019-07-14 22:17                   ` Sebastian Gniazdowski
2019-07-15  8:54                     ` Peter Stephenson
2019-07-15  9:29           ` Peter Stephenson
2019-07-15 18:45             ` Sebastian Gniazdowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180916185117.1c0eaba1@pws-HP.localdomain \
    --to=p.w.stephenson@ntlworld.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).