Skip to content

Commit

Permalink
Fix most of job control (-m/-o monitor) in scripts
Browse files Browse the repository at this point in the history
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: #119
  • Loading branch information
McDutchie committed Feb 12, 2021
1 parent 37a18ba commit 41ebb55
Show file tree
Hide file tree
Showing 8 changed files with 125 additions and 56 deletions.
5 changes: 5 additions & 0 deletions NEWS
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/bltins/misc.c
Expand Up @@ -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);
Expand Down
5 changes: 0 additions & 5 deletions src/cmd/ksh93/bltins/typeset.c
Expand Up @@ -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;
Expand All @@ -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*/
Expand Down
5 changes: 5 additions & 0 deletions src/cmd/ksh93/sh/args.c
Expand Up @@ -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))
{
Expand Down
81 changes: 46 additions & 35 deletions src/cmd/ksh93/sh/jobs.c
Expand Up @@ -27,6 +27,7 @@
* Written October, 1982
* Rewritten April, 1988
* Revised January, 1992
* Mended February, 2021
*/

#include "defs.h"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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 */
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1557,7 +1568,7 @@ int job_wait(register pid_t pid)
}
#endif /* SIGTSTP */
}
else
else if(job.jobcontrol)
{
if(pw->p_pid == tcgetpgrp(JOBTTY))
{
Expand Down
2 changes: 0 additions & 2 deletions src/cmd/ksh93/sh/subshell.c
Expand Up @@ -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;
Expand Down
18 changes: 8 additions & 10 deletions src/cmd/ksh93/sh/xec.c
Expand Up @@ -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);
Expand All @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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 */
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 41ebb55

Please sign in to comment.