1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-02-14 20:22:21 +00:00

Fix bad job control msg on trapped SIGINT and set -b (re: fc655f1)

When running an external command while trapping Ctrl+C via SIGINT,
and set -b is on, then a spurious Done job control message is
printed. No background job was executed.

    $ trap 'ls' INT
    $ set -b
    $ <Ctrl+C>[file listing follows]

    [1] +  Done                    set -b

In jobs.c (487-493), job_reap() calls job_list() to list a running
or completed background job, passing the JOB_NFLAG bit to only
print jobs with the P_NOTIFY flag. But the 'ls' in the trap is not
a background job. So it is getting the P_NOTIFY flag by mistake.

In fact all processes get the P_NOTIFY flag by default when they
terminate. Somehow the shell normally does not follow a code path
that calls job_list() for foreground processes, but does when
running one from a trap. I have not yet figured out how that works.

What I do know is that there is no reason why non-background
processes should ever have the P_NOTIFY flag set on termination,
because those should never print any 'Done' messages. And we seem
to have a handy P_BG flag that is set for background processes; we
can check for this before setting P_NOTIFY. The only thing is that
flag is only compiled in if SHOPT_BGX is enabled, as it was added
to support that functionality.

For some reason I am unable to reproduce the bug in a pty session,
so there is no pty.sh regression test.

src/cmd/ksh93/sh/jobs.c:
- Rename misleadingly named P_FG flag to P_MOVED2FG; this flag is
  not set for all foreground processes but only for processes moved
  to the foreground by job_switch(), called by the fg command.
- Compile in the P_BG flag even when SHOPT_BGX is not enabled. We
  need to set this flag to check for a background job.
- job_reap(): Do not set the P_NOTIFY flag for all terminated
  processes, but only for those with P_BG set.

src/cmd/ksh93/sh/xec.c: sh_fork():
- Also pass special argument 1 for background job to job_post() if
  SHOPT_BGX is not enabled. This is what gets it to set P_BG.
- Simplify 5 lines of convoluted code into 1.

Resolves: https://github.com/ksh93/ksh/issues/481
This commit is contained in:
Martijn Dekker 2022-07-14 05:51:17 +02:00
parent ffee9100d5
commit adc6a64b82
4 changed files with 28 additions and 37 deletions

5
NEWS
View file

@ -3,6 +3,11 @@ For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0
Any uppercase BUG_* names are modernish shell bug IDs.
2022-07-14:
- Fixed a bug that caused a spurious "Done" message on the interactive shell
when an external command was run as a foreground job from a SIGINT trap.
2022-07-12:
- The .sh.level variable can now only be changed within a DEBUG trap. When

View file

@ -23,7 +23,7 @@
#define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */
#define SH_RELEASE_SVER "1.0.0-beta.2" /* semantic version number: https://semver.org */
#define SH_RELEASE_DATE "2022-07-12" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_DATE "2022-07-14" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_CPYR "(c) 2020-2022 Contributors to ksh " SH_RELEASE_FORK
/* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */

View file

@ -153,10 +153,8 @@ struct back_save
#define P_DONE 040
#define P_COREDUMP 0100
#define P_DISOWN 0200
#define P_FG 0400
#if SHOPT_BGX
#define P_BG 01000
#endif /* SHOPT_BGX */
#define P_MOVED2FG 0400 /* set if the process was moved to the foreground by job_switch() */
#define P_BG 01000 /* set if the process is running in the background */
static int job_chksave(pid_t);
static struct process *job_bypid(pid_t);
@ -405,9 +403,12 @@ int job_reap(register int sig)
sh_subjobcheck(pid);
pw->p_flag &= ~(P_STOPPED|P_SIGNALLED);
pw->p_flag |= P_DONE;
if(pw->p_flag&P_BG)
pw->p_flag |= P_NOTIFY;
if (WIFSIGNALED(wstat))
{
pw->p_flag |= (P_DONE|P_NOTIFY|P_SIGNALLED);
pw->p_flag |= P_SIGNALLED;
if (WTERMCORE(wstat))
pw->p_flag |= P_COREDUMP;
pw->p_exit = WTERMSIG(wstat);
@ -424,13 +425,12 @@ int job_reap(register int sig)
}
else
{
pw->p_flag |= (P_DONE|P_NOTIFY);
pw->p_exit = pw->p_exitmin;
if(WEXITSTATUS(wstat) > pw->p_exitmin)
pw->p_exit = WEXITSTATUS(wstat);
}
#if SHOPT_BGX
if((pw->p_flag&P_DONE) && (pw->p_flag&P_BG))
if(pw->p_flag&P_BG)
{
job.numbjob--;
if(sh.st.trapcom[SIGCHLD])
@ -786,7 +786,7 @@ static void job_reset(register struct process *pw)
return;
#endif /* SIGTSTP */
/* force the following tty_get() to do a tcgetattr() unless fg */
if(!(pw->p_flag&P_FG))
if(!(pw->p_flag&P_MOVED2FG))
tty_set(-1, 0, NIL(struct termios*));
if(pw && (pw->p_flag&P_SIGNALLED) && pw->p_exit!=SIGHUP)
{
@ -1206,18 +1206,16 @@ void job_clear(void)
}
/*
* put the process <pid> on the process list and return the job number
* if non-zero, <join> is the process ID of the job to join
* Put the process <pid> on the process list and return the job number.
* If <join> is 1, the job is marked as a background job (P_BG);
* otherwise, if non-zero, <join> is the process ID of the job to join.
*/
int job_post(pid_t pid, pid_t join)
{
register struct process *pw;
register History_t *hp = sh.hist_ptr;
#if SHOPT_BGX
int val,bg=0;
#else
int val;
#endif
char bg = 0;
sh.jobenv = sh.curenv;
if(job.toclear)
{
@ -1225,14 +1223,14 @@ int job_post(pid_t pid, pid_t join)
return(0);
}
job_lock();
#if SHOPT_BGX
if(join==1)
{
join = 0;
bg = P_BG;
bg++;
#if SHOPT_BGX
job.numbjob++;
}
#endif /* SHOPT_BGX */
}
if(njob_savelist < NJOB_SAVELIST)
init_savelist();
if(pw = job_bypid(pid))
@ -1316,15 +1314,15 @@ int job_post(pid_t pid, pid_t join)
else
pw->p_flag |= (P_DONE|P_NOTIFY);
}
#if SHOPT_BGX
if(bg)
{
if(pw->p_flag&P_DONE)
job.numbjob--;
else
if(!(pw->p_flag&P_DONE))
pw->p_flag |= P_BG;
}
#if SHOPT_BGX
else
job.numbjob--;
#endif /* SHOPT_BGX */
}
lastpid = 0;
job_unlock();
return(pw->p_job);
@ -1608,9 +1606,7 @@ int job_switch(register struct process *pw,int bgflag)
{
sfprintf(outfile,"[%d]\t",(int)pw->p_job);
sh.bckpid = pw->p_pid;
#if SHOPT_BGX
pw->p_flag |= P_BG;
#endif
msg = "&";
}
else
@ -1631,10 +1627,8 @@ int job_switch(register struct process *pw,int bgflag)
return(1);
}
job.waitall = 1;
pw->p_flag |= P_FG;
#if SHOPT_BGX
pw->p_flag |= P_MOVED2FG;
pw->p_flag &= ~P_BG;
#endif
job_wait(pw->p_pid);
job.waitall = 0;
}

View file

@ -2887,15 +2887,7 @@ pid_t _sh_fork(register pid_t parent,int flags,int *jobid)
sh.cpid = parent;
if(!postid && job.curjobid && (flags&FPOU))
postid = job.curpgid;
#if SHOPT_BGX
if(!postid && (flags&(FAMP|FINT)) == (FAMP|FINT))
postid = 1;
myjob = job_post(parent,postid);
if(postid==1)
postid = 0;
#else
myjob = job_post(parent,postid);
#endif /* SHOPT_BGX */
myjob = job_post(parent, (!postid && (flags&(FAMP|FINT))==(FAMP|FINT)) ? 1 : postid);
if(job.waitall && (flags&FPOU))
{
if(!job.curjobid)