Skip to content

Commit

Permalink
Fix crash: do not list job if in 60 sec grace period (re: 3385868)
Browse files Browse the repository at this point in the history
The crash in job_list() or job_unpost() could still occur after the
previous patch if a signal was being handled after $TMOUT was
exceeded and the 60-second grace period was entered.

It *should* work to add a general check for !sh_isstate(SH_GRACE).
We know that the SH_GRACE state is set immediately after printing
the 60 second grace period warning message:
https://github.com/ksh93/ksh/blob/9de65210/src/cmd/ksh93/sh/io.c#L1869-L1870
(and that the crashes occur upon re-evaluating the $PS1 prompt
after setting the SH_GRACE state). We know that the SH_GRACE state
is not turned off again until either the user enters a line:
https://github.com/ksh93/ksh/blob/9de65210/src/cmd/ksh93/sh/main.c#L474
or the shell times out after the grace period:
https://github.com/ksh93/ksh/blob/9de65210/src/cmd/ksh93/sh/io.c#L1861
The SH_GRACE state flag is not used or changed in any other context
(verified with grep -rn SH_GRACE src/cmd/ksh93). So, logically,
this should suffice to make sure the crash stays gone.

src/cmd/ksh93/sh/jobs.c: job_reap():
- Do not list jobs when the SH_GRACE state (the 60 second timeout
  grace period after TMOUT was exceeded) is active.
- Keep the previous check for job control just to be sure, and
  because it makes sense.

Fixes: #103 (again)
  • Loading branch information
McDutchie committed Aug 7, 2020
1 parent 9de6521 commit e805c7d
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 4 deletions.
7 changes: 5 additions & 2 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@ For full details, see the git log at: https://github.com/ksh93/ksh

Any uppercase BUG_* names are modernish shell bug IDs.

2020-08-06:
2020-08-07:

- Fixed a crash that occurred intermittently if 'set -b'/'set -o notify' is
- Fixed a crash that occurred intermittently when entering the 60 second
timeout grace period after exceeding $TMOUT, if 'set -b'/'set -o notify' is
active and $PS1 contains a command substitution running an external command.

2020-08-06:

- Added the '${.sh.pid}' variable as an alternative to Bash's '$BASHPID'.
This variable is set to the current shell's PID, unlike '$$' (which is
set to the parent shell's PID). In virtual subshells '${.sh.pid}' is not
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@
* David Korn <[email protected]> *
* *
***********************************************************************/
#define SH_RELEASE "93u+m 2020-08-06"
#define SH_RELEASE "93u+m 2020-08-07"
2 changes: 1 addition & 1 deletion src/cmd/ksh93/sh/jobs.c
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ int job_reap(register int sig)
nochild = 1;
}
shp->gd->waitevent = waitevent;
if(job.jobcontrol && sh_isoption(SH_NOTIFY) && sh_isstate(SH_TTYWAIT))
if(job.jobcontrol && sh_isoption(SH_NOTIFY) && sh_isstate(SH_TTYWAIT) && !sh_isstate(SH_GRACE))
{
outfile = sfstderr;
job_list(pw,JOB_NFLAG|JOB_NLFLAG);
Expand Down

2 comments on commit e805c7d

@JohnoKing
Copy link

@JohnoKing JohnoKing commented on e805c7d Aug 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix causes set -o notify to break when a timeout is specified:

$ set -o notify; sleep 2 & TMOUT=1 # This works in commit 9de65210
[1]	29676
$ 
shell will timeout in 60 seconds due to inactivity
$ # Done message isn't shown unless the Enter key is pressed, which is a regression

@McDutchie
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reopening #103.

Please sign in to comment.