From 37a18bab7173427ce045c9a67772cb98eb678cb7 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Thu, 11 Feb 2021 13:41:10 +0000 Subject: [PATCH] Fix ${ comsub; } killing job control Another longstanding whopper of a bug in basic ksh93 functionality: run a ${ shared-state; } command substitution twice and job control promptly loses track of all your running jobs. New jobs are tracked again until you run another two shared-state command substitutions. This is in at least 93t+, 93u-, 93u+, 93v- and ksh2020. $ sleep 300 & [1] 56883 $ jobs # OK [1] + Running sleep 300 & $ v=${ echo hi1; } $ jobs # OK [1] + Running sleep 300 & $ v=${ echo hi2; } $ jobs # Nothing! $ fg ksh: fg: no such job src/cmd/ksh93/sh/subshell.c: sh_subshell(): - The current environment number shp->curenv (a.k.a. sh.curenv) was not being restored if the virtual subshell we're leaving is of the shared-state command substitution variety as it was wrongly considered to be part of the environment that didn't need restoring. This caused it to be out of sync with shp->jobenv (a.k.a. sh.jobenv) which did get restored from savedcurenv. Restore both from savedcurenv at the same time for any subshell. (How these numbers are used exactly remains to be discovered.) src/cmd/ksh93/tests/jobs.sh: - Added, with a test for this bug to start it off. There is no other test script where job control fits, and a lot more related fixes are anticipated: https://github.com/ksh93/ksh/issues/119 --- NEWS | 5 ++++ src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/subshell.c | 3 +-- src/cmd/ksh93/tests/jobs.sh | 44 +++++++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 3 deletions(-) create mode 100755 src/cmd/ksh93/tests/jobs.sh diff --git a/NEWS b/NEWS index ddef1a504..2db47c538 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,11 @@ For full details, see the git log at: https://github.com/ksh93/ksh Any uppercase BUG_* names are modernish shell bug IDs. +2021-02-11: + +- 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. + 2021-02-05: - Fixed a longstanding bug that caused redirections that store a file diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index d51aa3a19..ffc1fd2c2 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -20,7 +20,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.0.0-alpha" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2021-02-05" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2021-02-11" /* must be in this format for $((.sh.version)) */ #define SH_RELEASE_CPYR "(c) 2020-2021 Contributors to ksh " SH_RELEASE_FORK /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */ diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index 04053f496..e48a242f9 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -752,7 +752,7 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub) path_delete((Pathcomp_t*)shp->pathlist); shp->pathlist = (void*)sp->pathlist; job_subrestore(sp->jobs); - shp->jobenv = savecurenv; + shp->curenv = shp->jobenv = savecurenv; job.curpgid = savejobpgid; job.exitval = saveexitval; shp->bckpid = sp->bckpid; @@ -794,7 +794,6 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub) if(n>0) memset(&shp->st.trapcom[savst.trapmax],0,n*sizeof(char*)); shp->st = savst; - shp->curenv = savecurenv; shp->st.otrap = 0; if(nsig) { diff --git a/src/cmd/ksh93/tests/jobs.sh b/src/cmd/ksh93/tests/jobs.sh new file mode 100755 index 000000000..f8fe9adc3 --- /dev/null +++ b/src/cmd/ksh93/tests/jobs.sh @@ -0,0 +1,44 @@ +######################################################################## +# # +# This file is part of the ksh 93u+m package # +# Copyright (c) 2021 Contributors to ksh 93u+m # +# # +# and is licensed under the # +# Eclipse Public License, Version 1.0 # +# # +# A copy of the License is available at # +# http://www.eclipse.org/org/documents/epl-v10.html # +# (with md5 checksum b35adb5213ca9657e911e9befb180842) # +# # +# Martijn Dekker # +# # +######################################################################## +function err_exit +{ + print -u2 -n "\t" + print -u2 -r "${Command}[$1]: ${@:2}" + let Errors++ +} +alias err_exit='err_exit $LINENO' + +Command=${0##*/} +integer Errors=0 + +[[ -d $tmp && -w $tmp && $tmp == "$PWD" ]] || { err\_exit "$LINENO" '$tmp not set; run this from shtests. Aborting.'; exit 1; } + +# All the tests here should run with job control on +set -o monitor + +# ====== +# 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=$! +j1=${ jobs; } +[[ $j1 == $'[2] + Running '*$'\n[1] - Running '* ]] || err_exit "sleep jobs not registered (got $(printf %q "$j1"))" +: ${ :; } ${ :; } +j2=${ jobs; } +kill $p1 $p2 +[[ $j2 == "$j1" ]] || err_exit "jobs lost after shared-state command substitution ($(printf %q "$j2") != $(printf %q "$j1"))" + +# ====== +exit $((Errors<125?Errors:125))