Fix most of job control (-m/-o monitor) in scripts

If I haven't missed anything, this should make the non-interactive
aspects of job control in scripts work as expected, except for the
"<command unknown>" issue in the output of 'bg', 'fg' and 'jobs'
(which is not such a high priority as those commands are really
designed for interactive use).

Plus, I believe I now finally understand what these three are for:
* The job.jobcontrol variable is set to nonzero by job_init() in
  jobs.c if, and only if, the shell is interactive *and* managed to
  get control of the terminal. Therefore, any changing of terminal
  settings (tcsetpgrp(3), tty_set()) should only be done if
  job.jobcontrol is nonzero. This commit changes several checks for
  sh_isoption(SH_INTERACTIVE) to checks for job.jobcontrol for
  better consistency with this.
* The state flag, sh_isstate(SH_MONITOR), determines whether the
  bits of job control that are relevant for both scripts and
  interactive shells are active, which is mostly making sure that a
  background job gets its own process group (setpgid(3)).
* The shell option, sh_isoption(SH_MONITOR), is just that. When the
  user turns it on or off, the state flag is synched with it. It
  should usually not be directly checked for, as the state may be
  temporarily turned off without turning off the option.

Prior discussion:
https://www.mail-archive.com/austin-group-l@opengroup.org/msg06456.html

src/cmd/ksh93/bltins/typeset.c, src/cmd/ksh93/sh/args.c:
- Move synching the SH_MONITOR state flag with the SH_MONITOR
  shell option from b_set() (the 'set' builtin) to sh_applyopts()
  which is indirectly called from b_set() and is also used when
  parsing the shell invocation command line. This ensures -m is
  properly enabled in both scenarios.

src/cmd/ksh93/sh/jobs.c:
- job_init(): Do not refuse to initialise job control on
  non-interactive shells. Instead, skip everything that should only
  be done on interactive shells (i.e., everything to do with the
  terminal). This function is now even more of a mess than it was
  before, so refactoring may be desirabe at some point.
- job_close(), job_set(), job_reset(), job_wait(): Do not reset the
  terminal process group (tcsetpgrp()) if job.jobcontrol isn't on.

src/cmd/ksh93/sh/xec.c:
- sh_exec(): TFORK: For SIGINT handling, check the SH_MONITOR
  state flag, not the shell option.
- sh_exec(): TFORK: Do not turn off the SH_MONITOR state flag in
  forked children. The non-interactive part of job control should
  stay active. Instead, turn off the SH_INTERACTIVE state flag so
  we don't get interactive shell behaviour (i.e. job control noise
  on the terminal) in forked subshells.
- _sh_fork(), sh_ntfork(): Do not reset the terminal process group
  (tcsetpgrp()) if job.jobcontrol isn't on. Do not turn off the
  SH_MONITOR state flag in forked children.

src/cmd/ksh93/sh/subshell.c: sh_subfork():
- Do not turn off the monitor option and state in forked subshells.
  The non-interactive part of job control should stay active.

src/cmd/ksh93/bltins/misc.c: b_bg():
- Check isstate(SH_MONITOR) instead of sh_isoption(SH_MONITOR) &&
  job.jobcontrol before throwing a 'no job control' error.
  This fixes a minor bug: fg, bg and disown could quietly fail.

src/cmd/ksh93/tests/jobs.sh:
- Add tests for 'fg' with job control IDs (%%, %1) in scripts.
- Add test checking that a background job launched from a subsell
  with job control enabled correctly becomes the leader of its own
  process group.

Makes progress on: https://github.com/ksh93/ksh/issues/119

NEWS

@@ -8,6 +8,11 @@ Any uppercase BUG_* names are modernish shell bug IDs.
 - Fixed a bug that caused ksh to lose track of all running background jobs if
   a shared-state command substitution of the form v=${ cmd; } was used twice.
 
+- Job control (the -m/-o monitor option) has been fixed for scripts. Background
+  jobs are now correctly assigned their own process group when run from
+  subshells (except command substitutions). The 'fg' command now also works for
+  scripts as it does on other shells, though 'wait' should be preferred.
+
 2021-02-05:
 
 - Fixed a longstanding bug that caused redirections that store a file

src/cmd/ksh93/bltins/misc.c

@@ -416,7 +416,7 @@ int    b_bg(register int n,register char *argv[],Shbltin_t *context)
 	if(error_info.errors)
 		errormsg(SH_DICT,ERROR_usage(2),"%s",optusage((char*)0));
 	argv += opt_info.index;
-	if(!sh_isoption(SH_MONITOR) || !job.jobcontrol)
+	if(!sh_isstate(SH_MONITOR))
 	{
 		errormsg(SH_DICT,ERROR_exit(1),e_no_jctl);
 		return(1);

src/cmd/ksh93/bltins/typeset.c

@@ -1144,7 +1144,6 @@ int	b_builtin(int argc,char *argv[],Shbltin_t *context)
 int    b_set(int argc,register char *argv[],Shbltin_t *context)
 {
 	struct tdata tdata;
-	int was_monitor = sh_isoption(SH_MONITOR);
 	memset(&tdata,0,sizeof(tdata));
 	tdata.sh = context->shp;
 	tdata.prefix=0;
@@ -1156,10 +1155,6 @@ int    b_set(int argc,register char *argv[],Shbltin_t *context)
 			sh_onstate(SH_VERBOSE);
 		else
 			sh_offstate(SH_VERBOSE);
-		if(sh_isoption(SH_MONITOR) && !was_monitor)
-			sh_onstate(SH_MONITOR);
-		else if(!sh_isoption(SH_MONITOR)  && was_monitor)
-			sh_offstate(SH_MONITOR);
 	}
 	else
 		/*scan name chain and print*/

src/cmd/ksh93/sh/args.c

@@ -360,6 +360,11 @@ void sh_applyopts(Shell_t* shp,Shopt_t newflags)
 			(shp->gd->userid==shp->gd->euserid && shp->gd->groupid==shp->gd->egroupid))
 				off_option(&newflags,SH_PRIVILEGED);
 	}
+	/* sync monitor (part of job control) state with -o monitor option change */
+	if(!sh_isoption(SH_MONITOR) && is_option(&newflags,SH_MONITOR))
+		sh_onstate(SH_MONITOR);
+	else if(sh_isoption(SH_MONITOR) && !is_option(&newflags,SH_MONITOR))
+		sh_offstate(SH_MONITOR);
 	/* -o posix also switches -o braceexpand and -o letoctal */
 	if(!sh_isoption(SH_POSIX) && is_option(&newflags,SH_POSIX))
 	{

src/cmd/ksh93/sh/jobs.c

@@ -27,6 +27,7 @@
  *  Written October, 1982
  *  Rewritten April, 1988
  *  Revised January, 1992
+ *  Mended February, 2021
  */
 
 #include	"defs.h"
@@ -436,11 +437,11 @@ int job_reap(register int sig)
 		/* only top-level process in job should have notify set */
 		if(px && pw != px)
 			pw->p_flag &= ~P_NOTIFY;
-		if(pid==pw->p_fgrp && pid==tcgetpgrp(JOBTTY))
+		if(job.jobcontrol && pid==pw->p_fgrp && pid==tcgetpgrp(JOBTTY))
 		{
 			px = job_byjid((int)pw->p_job);
 			for(; px && (px->p_flag&P_DONE); px=px->p_nxtproc);
-			if(!px && sh_isoption(SH_INTERACTIVE))
+			if(!px)
 				tcsetpgrp(JOBTTY,job.mypid);
 		}
 #if !SHOPT_BGX
@@ -500,26 +501,27 @@ void job_init(Shell_t *shp, int lflag)
 #   endif
 	if(njob_savelist < NJOB_SAVELIST)
 		init_savelist();
-	if(!sh_isoption(SH_INTERACTIVE))
-		return;
 	/* use new line discipline when available */
+	if(sh_isoption(SH_INTERACTIVE))
+	{
 #ifdef NTTYDISC
 #   ifdef FIOLOOKLD
-	if((job.linedisc = ioctl(JOBTTY, FIOLOOKLD, 0)) <0)
+		if((job.linedisc = ioctl(JOBTTY, FIOLOOKLD, 0)) <0)
 #   else
-	if(ioctl(JOBTTY,TIOCGETD,&job.linedisc) !=0)
+		if(ioctl(JOBTTY,TIOCGETD,&job.linedisc) !=0)
 #   endif /* FIOLOOKLD */
-		return;
-	if(job.linedisc!=NTTYDISC && job.linedisc!=OTTYDISC)
-	{
-		/* no job control when running with MPX */
+			return;
+		if(job.linedisc!=NTTYDISC && job.linedisc!=OTTYDISC)
+		{
+			/* no job control when running with MPX */
 #   if SHOPT_VSH
-		sh_onoption(SH_VIRAW);
+			sh_onoption(SH_VIRAW);
 #   endif /* SHOPT_VSH */
-		return;
+			return;
+		}
+		if(job.linedisc==NTTYDISC)
+			job.linedisc = -1;
 	}
-	if(job.linedisc==NTTYDISC)
-		job.linedisc = -1;
 #endif /* NTTYDISC */
 
 	job.mypgid = getpgrp();
@@ -530,29 +532,30 @@ void job_init(Shell_t *shp, int lflag)
 		/* This should have already been done by rlogin */
                 register int fd;
                 register char *ttynam;
-		int err = errno;
 #ifndef SIGTSTP
                 setpgid(0,shp->gd->pid);
 #endif /*SIGTSTP */
-                if(job.mypgid<0 || !(ttynam=ttyname(JOBTTY)))
-                        return;
-		while(close(JOBTTY)<0 && errno==EINTR)
-			errno = err;
-                if((fd = open(ttynam,O_RDWR)) <0)
-                        return;
-                if(fd!=JOBTTY)
-                        sh_iorenumber(shp,fd,JOBTTY);
-                job.mypgid = shp->gd->pid;
+		if(sh_isoption(SH_INTERACTIVE))
+		{
+			if(job.mypgid<0 || !(ttynam=ttyname(JOBTTY)))
+				return;
+			while(close(JOBTTY)<0 && errno==EINTR)
+				;
+			if((fd = open(ttynam,O_RDWR)) <0)
+				return;
+			if(fd!=JOBTTY)
+				sh_iorenumber(shp,fd,JOBTTY);
 #ifdef SIGTSTP
-                tcsetpgrp(JOBTTY,shp->gd->pid);
-                setpgid(0,shp->gd->pid);
+			tcsetpgrp(JOBTTY,shp->gd->pid);
 #endif /* SIGTSTP */
+		}
+                job.mypgid = shp->gd->pid;
         }
 #ifdef SIGTSTP
-	if(possible = (setpgid(0,job.mypgid)>=0) || errno==EPERM)
+	possible = (setpgid(0,job.mypgid) >= 0);
+	if(sh_isoption(SH_INTERACTIVE) && (possible || errno==EPERM))
 	{
 		/* wait until we are in the foreground */
-
 		while((job.mytgid=tcgetpgrp(JOBTTY)) != job.mypgid)
 		{
 			if(job.mytgid <= 0)
@@ -572,7 +575,7 @@ void job_init(Shell_t *shp, int lflag)
 
 #ifdef NTTYDISC
 	/* set the line discipline */
-	if(job.linedisc>=0)
+	if(sh_isoption(SH_INTERACTIVE) && job.linedisc>=0)
 	{
 		int linedisc = NTTYDISC;
 #   ifdef FIOPUSHLD
@@ -593,14 +596,17 @@ void job_init(Shell_t *shp, int lflag)
 			errormsg(SH_DICT,0,e_newtty);
 		else
 			job.linedisc = -1;
-	}
 #endif /* NTTYDISC */
+	}
 	if(!possible)
 		return;
 
 #ifdef SIGTSTP
 	/* make sure that we are a process group leader */
 	setpgid(0,shp->gd->pid);
+	job.mypid = shp->gd->pid;
+	if(!sh_isoption(SH_INTERACTIVE))
+		return;
 #   if defined(SA_NOCLDSTOP) || defined(SA_NOCLDWAIT)
 #   	if !defined(SA_NOCLDSTOP)
 #	    define SA_NOCLDSTOP	0
@@ -627,7 +633,6 @@ void job_init(Shell_t *shp, int lflag)
 #   endif /* CNSUSP */
 	sh_onoption(SH_MONITOR);
 	job.jobcontrol++;
-	job.mypid = shp->gd->pid;
 #endif /* SIGTSTP */
 	return;
 }
@@ -677,7 +682,7 @@ int job_close(Shell_t* shp)
 	}
 	job_unlock();
 #   ifdef SIGTSTP
-	if(possible && setpgid(0,job.mypgid)>=0)
+	if(job.jobcontrol && setpgid(0,job.mypgid)>=0)
 		tcsetpgrp(job.fd,job.mypgid);
 #   endif /* SIGTSTP */
 #   ifdef NTTYDISC
@@ -717,6 +722,8 @@ int job_close(Shell_t* shp)
 static void job_set(register struct process *pw)
 {
 	Shell_t *shp = pw->p_shp;
+	if(!job.jobcontrol)
+		return;
 	/* save current terminal state */
 	tty_get(job.fd,&my_stty);
 	if(pw->p_flag&P_STTY)
@@ -739,9 +746,13 @@ static void job_reset(register struct process *pw)
 	/* save the terminal state for current job */
 #ifdef SIGTSTP
 	pid_t tgrp;
+#endif
+	if(!job.jobcontrol)
+		return;
+#ifdef SIGTSTP
 	if((tgrp=tcgetpgrp(job.fd))!=job.mypid)
 		job_fgrp(pw,tgrp);
-	if(sh_isoption(SH_INTERACTIVE) && tcsetpgrp(job.fd,job.mypid) !=0)
+	if(tcsetpgrp(job.fd,job.mypid) !=0)
 		return;
 #endif	/* SIGTSTP */
 	/* force the following tty_get() to do a tcgetattr() unless fg */
@@ -1479,7 +1490,7 @@ int	job_wait(register pid_t pid)
 			if(pw->p_flag&P_STOPPED)
 			{
 				pw->p_flag |= P_EXITSAVE;
-				if(sh_isoption(SH_INTERACTIVE) && !sh_isstate(SH_FORKED))
+				if(job.jobcontrol && !sh_isstate(SH_FORKED))
 				{
 					if( pw->p_exit!=SIGTTIN && pw->p_exit!=SIGTTOU)
 						break;
@@ -1557,7 +1568,7 @@ int	job_wait(register pid_t pid)
 		}
 #endif /* SIGTSTP */
 	}
-	else
+	else if(job.jobcontrol)
 	{
 		if(pw->p_pid == tcgetpgrp(JOBTTY))
 		{

src/cmd/ksh93/sh/subshell.c

@@ -207,8 +207,6 @@ void sh_subfork(void)
 		/* this is the child part of the fork */
 		/* setting subpid to 1 causes subshell to exit when reached */
 		sh_onstate(SH_FORKED);
-		sh_offoption(SH_MONITOR);
-		sh_offstate(SH_MONITOR);
 		subshell_data = 0;
 		shp->subshell = 0;
 		shp->comsub = 0;

src/cmd/ksh93/sh/xec.c

@@ -1627,7 +1627,7 @@ int sh_exec(register const Shnode_t *t, int flags)
 					shp->bckpid = parent;
 				else if(!(type&(FAMP|FPOU)))
 				{
-					if(!sh_isoption(SH_MONITOR))
+					if(!sh_isstate(SH_MONITOR))
 					{
 						if(!(shp->sigflag[SIGINT]&(SH_SIGFAULT|SH_SIGOFF)))
 							sh_sigtrap(SIGINT);
@@ -1645,7 +1645,7 @@ int sh_exec(register const Shnode_t *t, int flags)
 						sh_iorestore(shp,topfd,0);
 					if(usepipe && tsetio &&  subdup && unpipe)
 						sh_iounpipe(shp);
-					if(!sh_isoption(SH_MONITOR))
+					if(!sh_isstate(SH_MONITOR))
 					{
 						shp->trapnote &= ~SH_SIGIGNORE;
 						if(shp->exitval == (SH_EXITSIG|SIGINT))
@@ -1692,7 +1692,7 @@ int sh_exec(register const Shnode_t *t, int flags)
 							sh_chkopen(e_devnull);
 					}
 				}
-				sh_offstate(SH_MONITOR);
+				sh_offstate(SH_INTERACTIVE);
 				/* pipe in or out */
 #ifdef _lib_nice
 				if((type&FAMP) && sh_isoption(SH_BGNICE))
@@ -1881,7 +1881,7 @@ int sh_exec(register const Shnode_t *t, int flags)
 				sh_done(shp,0);
 			}
 			else if(((type=t->par.partre->tre.tretyp)&FAMP) && ((type&COMMSK)==TFORK)
-			&& !sh_isoption(SH_INTERACTIVE) && !job.jobcontrol)
+			&& !sh_isoption(SH_INTERACTIVE))
 			{
 				/* Optimize '( simple_command & )' */
 				pid_t	pid;
@@ -2005,7 +2005,7 @@ int sh_exec(register const Shnode_t *t, int flags)
 			}
 			shp->exitval = n;
 #ifdef SIGTSTP
-			if(!pipejob && sh_isstate(SH_MONITOR) && sh_isoption(SH_INTERACTIVE))
+			if(!pipejob && sh_isstate(SH_MONITOR) && job.jobcontrol)
 				tcsetpgrp(JOBTTY,shp->gd->pid);
 #endif /*SIGTSTP */
 			job.curpgid = savepgid;
@@ -2912,8 +2912,6 @@ pid_t _sh_fork(Shell_t *shp,register pid_t parent,int flags,int *jobid)
 	shp->gd->nforks=0;
 	timerdel(NIL(void*));
 #ifdef JOBS
-	if(!job.jobcontrol && !(flags&FAMP))
-		sh_offstate(SH_MONITOR);
 	if(sh_isstate(SH_MONITOR))
 	{
 		parent = shgd->current_pid;
@@ -2922,7 +2920,7 @@ pid_t _sh_fork(Shell_t *shp,register pid_t parent,int flags,int *jobid)
 		while(setpgid(0,job.curpgid)<0 && job.curpgid!=parent)
 			job.curpgid = parent;
 #   ifdef SIGTSTP
-		if(job.curpgid==parent &&  !(flags&FAMP))
+		if(job.jobcontrol && job.curpgid==parent && !(flags&FAMP))
 			tcsetpgrp(job.fd,job.curpgid);
 #   endif /* SIGTSTP */
 	}
@@ -3634,7 +3632,7 @@ static pid_t sh_ntfork(Shell_t *shp,const Shnode_t *t,char *argv[],int *jobid,in
 #endif /* SIGTSTP */
 			if(spawnpid>0)
 				_sh_fork(shp,spawnpid,otype,jobid);
-			if(grp>0 && !(otype&FAMP))
+			if(job.jobcontrol && grp>0 && !(otype&FAMP))
 			{
 				while(tcsetpgrp(job.fd,job.curpgid)<0 && job.curpgid!=spawnpid)
 					job.curpgid = spawnpid;
@@ -3805,7 +3803,7 @@ static pid_t sh_ntfork(Shell_t *shp,const Shnode_t *t,char *argv[],int *jobid,in
 		if(grp==1)
 			job.curpgid = spawnpid;
 #   ifdef SIGTSTP
-		if(grp>0 && !(otype&FAMP))
+		if(job.jobcontrol && grp>0 && !(otype&FAMP))
 		{
 			while(tcsetpgrp(job.fd,job.curpgid)<0 && job.curpgid!=spawnpid)
 				job.curpgid = spawnpid;

src/cmd/ksh93/tests/jobs.sh

@@ -20,6 +20,7 @@ function err_exit
 	let Errors++
 }
 alias err_exit='err_exit $LINENO'
+alias warning='err\_exit $((Errors--,LINENO))'
 
 Command=${0##*/}
 integer Errors=0
@@ -29,15 +30,71 @@ integer Errors=0
 # All the tests here should run with job control on
 set -o monitor
 
+# ======
+# Check job control job IDs: %%, %n. Before 2021-02-11 this did not work for 'fg' in scripts.
+sleep 1 &
+kill %% >out 2>&1
+kill $! 2>/dev/null && err_exit "'kill %%' not working in script (got $(printf %q "$(<out)"))"
+sleep 1 &
+kill %2 >out 2>&1
+kill $! 2>/dev/null && err_exit "'kill %2' not working in script (got $(printf %q "$(<out)"))"
+sleep .05 &
+wait >out 2>&1
+kill $! 2>/dev/null && err_exit "'wait' not working in script (got $(printf %q "$(<out)"))"
+sleep .05 &
+wait %% >out 2>&1
+kill $! 2>/dev/null && err_exit "'wait %%' not working in script (got $(printf %q "$(<out)"))"
+sleep .05 &
+wait %1 >out 2>&1
+kill $! 2>/dev/null && err_exit "'wait %1' not working in script (got $(printf %q "$(<out)"))"
+sleep .05 &
+fg >out 2>&1
+kill $! 2>/dev/null && err_exit "'fg' not working in script (got $(printf %q "$(<out)"))"
+sleep .05 &
+fg %% >out 2>&1
+kill $! 2>/dev/null && err_exit "'fg %%' not working in script (got $(printf %q "$(<out)"))"
+sleep .05 &
+fg %1 >out 2>&1
+kill $! 2>/dev/null && err_exit "'fg %1' not working in script (got $(printf %q "$(<out)"))"
+sleep 1 &
+sleep 1 &
+bg >out 2>&1 || err_exit "'bg' not working in script (got $(printf %q "$(<out)"))"
+bg %% >out 2>&1 || err_exit "'bg %%' not working in script (got $(printf %q "$(<out)"))"
+bg %+ >out 2>&1 || err_exit "'bg %+' not working in script (got $(printf %q "$(<out)"))"
+bg %- >out 2>&1 || err_exit "'bg %-' not working in script (got $(printf %q "$(<out)"))"
+bg %1 >out 2>&1 || err_exit "'bg %1' not working in script (got $(printf %q "$(<out)"))"
+bg %2 >out 2>&1 || err_exit "'bg %2' not working in script (got $(printf %q "$(<out)"))"
+disown >out 2>&1 || err_exit "'disown' not working in script (got $(printf %q "$(<out)"))"
+disown %% >out 2>&1 || err_exit "'disown %%' not working in script (got $(printf %q "$(<out)"))"
+disown %+ >out 2>&1 || err_exit "'disown %+' not working in script (got $(printf %q "$(<out)"))"
+disown %- >out 2>&1 || err_exit "'disown %-' not working in script (got $(printf %q "$(<out)"))"
+disown %1 >out 2>&1 || err_exit "'disown %1' not working in script (got $(printf %q "$(<out)"))"
+disown %2 >out 2>&1 || err_exit "'disown %2' not working in script (got $(printf %q "$(<out)"))"
+kill %- >out 2>&1 || err_exit "'kill %-' not working in script (got $(printf %q "$(<out)"))"
+kill %+ >out 2>&1 || err_exit "'kill %+' not working in script (got $(printf %q "$(<out)"))"
+
+# fail gracefully: suppress "Terminated" noise on pre-93u+m ksh93
+{ wait; } 2>/dev/null
+
+# =====
+# Before 2021-02-11, job control was deactivated in subshells
+# https://www.mail-archive.com/austin-group-l@opengroup.org/msg06456.html
+(sleep 1 & UNIX95=1 ps -o pid=,pgid= -p $!) | IFS=$' \t' read -r pid pgid
+if	let "pid>0 && pgid>0" 2>/dev/null
+then	kill $pid
+	let "pgid == pid" || err_exit "background job run in subshell didn't get its own process group ($pgid != $pid)"
+else	warning "skipping subshell job control test due to non-compliant 'ps'"
+fi
+
 # ======
 # Before 2021-02-11, using a shared-state ${ command substitution; } twice caused ksh to lose track of all running jobs
-sleep 1 & p1=$!
-sleep 2 & p2=$!
+jobs >/dev/null  # get 'Done' messages out of the way
+sleep 1 & sleep 1 &
 j1=${ jobs; }
 [[ $j1 == $'[2] +  Running '*$'\n[1] -  Running '* ]] || err_exit "sleep jobs not registered (got $(printf %q "$j1"))"
 : ${ :; } ${ :; }
 j2=${ jobs; }
-kill $p1 $p2
+kill %- %+
 [[ $j2 == "$j1" ]] || err_exit "jobs lost after shared-state command substitution ($(printf %q "$j2") != $(printf %q "$j1"))"
 
 # ======