Re: Bug in sh emulation

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Sun, 11 Dec 2011 19:39:49 +0000
Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx> wrote:
> Not so much a follow up as a separate point...
> 
> If we're letting the subshell do job control (not resetting MONITOR in
> entersubsh() because POSIXJOBS is set), then presumably we shouldn't be
> resetting the signals that are special to shells that do job control?

Thinking a bit more...

Although it's not part of the problem here (we're in a real subshell),
we're leaving on job control in the case of POSIXJOBS too often... we
should only be doing it if we're entering a (...) subshell, not e.g. if
we're forking to execute an external command, and presumably also not if
we've been put into the background.  That hasn't been an issue
up to now because we've reset the signals any time MONITOR was
previously set.

So this modifies that patch only to do special things with job control
in subshells if we're about to enter a (...) subshell.

Not sure about $(...) etc.  Currently we're strict about not doing
anything MONITOR-related with those, so have left those turning MONITOR
off immediately.

I'm vaguely coming to the conclusion the SIGTTOU's are endemic because
we only do the attachtty() from the new process, which starts off in the
background, and hence the signal ignoring is a key feature, but that's a
good deal hazier.   Useful info at

http://www.gnu.org/s/hello/manual/libc/Implementing-a-Shell.html#Implementing-a-Shell

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.205
diff -p -u -r1.205 exec.c
--- Src/exec.c	26 Oct 2011 18:48:13 -0000	1.205
+++ Src/exec.c	11 Dec 2011 20:15:32 -0000
@@ -896,14 +896,16 @@ enum {
     /* Release the process group if pid is the shell's process group */
     ESUB_REVERTPGRP = 0x10,
     /* Don't handle the MONITOR option even if previously set */
-    ESUB_NOMONITOR = 0x20
+    ESUB_NOMONITOR = 0x20,
+    /* This is a subshell where job control is allowed */
+    ESUB_JOB_CONTROL = 0x40
 };
 
 /**/
 static void
 entersubsh(int flags)
 {
-    int sig, monitor;
+    int sig, monitor, job_control_ok;
 
     if (!(flags & ESUB_KEEPTRAP))
 	for (sig = 0; sig < VSIGCOUNT; sig++)
@@ -911,6 +913,7 @@ entersubsh(int flags)
 		sig != SIGDEBUG && sig != SIGZERR)
 		unsettrap(sig);
     monitor = isset(MONITOR);
+    job_control_ok = monitor && (flags & ESUB_JOB_CONTROL) && isset(POSIXJOBS);
     if (flags & ESUB_NOMONITOR)
 	opts[MONITOR] = 0;
     if (!isset(MONITOR)) {
@@ -959,7 +962,7 @@ entersubsh(int flags)
     if ((flags & ESUB_REVERTPGRP) && getpid() == mypgrp)
 	release_pgrp();
     shout = NULL;
-    if (isset(MONITOR)) {
+    if (!job_control_ok) {
 	signal_default(SIGTTOU);
 	signal_default(SIGTTIN);
 	signal_default(SIGTSTP);
@@ -971,7 +974,7 @@ entersubsh(int flags)
     }
     if (!(sigtrapped[SIGQUIT] & ZSIG_IGNORED))
 	signal_default(SIGQUIT);
-    if (!isset(POSIXJOBS))
+    if (!job_control_ok)
 	opts[MONITOR] = 0;
     opts[USEZLE] = 0;
     zleactive = 0;
@@ -2829,6 +2832,8 @@ execcmd(Estate state, int input, int out
 	flags = ((how & Z_ASYNC) ? ESUB_ASYNC : 0) | ESUB_PGRP;
 	if ((type != WC_SUBSH) && !(how & Z_ASYNC))
 	    flags |= ESUB_KEEPTRAP;
+	if (type == WC_SUBSH && !(how & Z_ASYNC))
+	    flags |= ESUB_JOB_CONTROL;
 	entersubsh(flags);
 	close(synch[1]);
 	forked = 1;


-- 
Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/