From 61437b2728cc879f46c772de94b803bff880a956 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Tue, 11 Aug 2020 01:02:31 +0100 Subject: [PATCH] Fix crash, take three (re: e805c7d9, 33858689) The current fix appears to be only partially successful in eliminating the intermittent crash, and also breaks '-o notify' during the 60-second $TMOUT grace period. This replaces it. The root cause appears to be that the state of job control becomes somehow inconsistent when running external commands in a command substitution expanded from the $PS1 prompt. The job_unpost() or (sometimes) the job_list() function intermittently crash. These are called if the SH_TTYWAIT state is active: https://github.com/ksh93/ksh/blob/88e8fa67/src/cmd/ksh93/sh/jobs.c#L463-L469 Temporarily deactivating the SSH_TTYWAIT state while expanding PS{1..4} prompts appears to fix the problem reliably. It is quite possible that this fix merely masks a bug in the job control system, but testing has shown that it stops ksh crashing without side effects, so I'm calling it good for now. Thanks to Marc Wilson for many hours of persistent testing. src/cmd/ksh93/sh/jobs.c: - Revert changes made in 33858689 and e805c7d9. src/cmd/ksh93/sh/io.c: io_prompt(): - Save SH_TTYWAIT state and turn it off while expanding prompts. Resolves: https://github.com/ksh93/ksh/issues/103 Resolves: https://github.com/ksh93/ksh/issues/112 --- NEWS | 9 +++------ src/cmd/ksh93/sh/io.c | 5 +++++ src/cmd/ksh93/sh/jobs.c | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/NEWS b/NEWS index a2e8dffd6..996c4f059 100644 --- a/NEWS +++ b/NEWS @@ -17,6 +17,9 @@ Any uppercase BUG_* names are modernish shell bug IDs. some unreserved ones (e.g. '~'). It now percent-encodes all characters except those 'unreserved' as per RFC3986 (ASCII alphanumeric plus -._~). +- Fixed a crash that occurred intermittently after running an external + command from a command substitution expanded from the $PS1 shell prompt. + 2020-08-09: - File name generation (a.k.a. pathname expansion, a.k.a. globbing) now @@ -32,12 +35,6 @@ Any uppercase BUG_* names are modernish shell bug IDs. error like 'redirect ls >foo.txt' now will not create 'foo.txt' and will not leave your standard output permanently redirected to it. -2020-08-07: - -- 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'. diff --git a/src/cmd/ksh93/sh/io.c b/src/cmd/ksh93/sh/io.c index f1e72f4b2..179bd3f88 100644 --- a/src/cmd/ksh93/sh/io.c +++ b/src/cmd/ksh93/sh/io.c @@ -2067,12 +2067,15 @@ static int io_prompt(Shell_t *shp,Sfio_t *iop,register int flag) char *endprompt; static short cmdno; int sfflags; + int was_ttywait_on; if(flag<3 && !sh_isstate(SH_INTERACTIVE)) flag = 0; if(flag==2 && sfpkrd(sffileno(iop),buff,1,'\n',0,1) >= 0) flag = 0; if(flag==0) return(sfsync(sfstderr)); + was_ttywait_on = sh_isstate(SH_TTYWAIT); + sh_offstate(SH_TTYWAIT); sfflags = sfset(sfstderr,SF_SHARE|SF_PUBLIC|SF_READ,0); if(!(shp->prompt=(char*)sfreserve(sfstderr,0,0))) shp->prompt = ""; @@ -2125,6 +2128,8 @@ static int io_prompt(Shell_t *shp,Sfio_t *iop,register int flag) done: if(*shp->prompt && (endprompt=(char*)sfreserve(sfstderr,0,0))) *endprompt = 0; + if(was_ttywait_on) + sh_onstate(SH_TTYWAIT); sfset(sfstderr,sfflags&SF_READ|SF_SHARE|SF_PUBLIC,1); return(sfsync(sfstderr)); } diff --git a/src/cmd/ksh93/sh/jobs.c b/src/cmd/ksh93/sh/jobs.c index c2436348c..3170276e0 100644 --- a/src/cmd/ksh93/sh/jobs.c +++ b/src/cmd/ksh93/sh/jobs.c @@ -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) && !sh_isstate(SH_GRACE)) + if(sh_isoption(SH_NOTIFY) && sh_isstate(SH_TTYWAIT)) { outfile = sfstderr; job_list(pw,JOB_NFLAG|JOB_NLFLAG);