Skip to content

Commit 50db00e

Browse files
committed
Fix subshell trap integrity, e.g. re-trapping a signal in subshell
Ksh handles local traps in virtual subshells the same way as local traps in ksh-style shell functions, which can cause incorrect operation. Reproducer script: trap 'echo "parent shell trap"; exit 0' USR1 (trap 'echo "subshell trap"' USR1; kill -USR1 $$) echo wrong Output on every ksh93 version: 'wrong' Output on every other shell: 'parent shell trap' The ksh93 output is wrong because $$ is the PID of the main shell, therefore 'kill -USR1 $$' from a subshell needs to issue SIGUSR1 to the main shell and trigger the 'echo SIGUSR1' trap. This is an inevitable consequence of processing signals in a virtual subshell. Signals are a process-wide property, but a virtual subshell and the parent shell share the same process. Therefore it is not possible to distinguish between the parent shell and subshell trap. This means virtual subshells are fundamentally incompatible with receiving signals. No workaround can make this work properly. Ksh could either assume the signal needs to be caught by the subshell trap (wrong in this case, but right in others) or by the parent shell trap. But it does neither and just gives up and does nothing, which I suppose is the least bad way of doing it wrong. As another example, consider a subshell that traps a signal, then passes its own process ID (as of 9de6521, that's ${.sh.pid}) to another process to say "here is where to signal me". A virtual subshell will send it the PID that it shares with the the parent shell. Even if a virtual subshell receives the signal correctly, it may fork mid-execution afterwards, depending on the commands that it runs (and this varies by implementation as we fix or work around bugs). So its PID could be invalidated at any time. Forking a virtual subshell at the time of trapping a signal is the only way to ensure a persistent PID and correct operation. src/cmd/ksh93/bltins/trap.c: b_trap(): - Fork when trapping (or ignoring) a signal in a virtual subshell. (There's no need to fork when trapping a pseudosignal.) src/cmd/ksh93/tests/signal.sh: - Add tests. These are simplified versions of tests already there, which issued 'kill' as a background job. Currently, running a background job causes a virtual subshell to fork before forking the 'kill' background job (or *any* background job, see e3d7bf1) -- an ugly partial workaround that I believe just became redundant and which I will remove in the next commit.
1 parent ed9053e commit 50db00e

4 files changed

Lines changed: 35 additions & 4 deletions

File tree

‎NEWS‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,15 @@ For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0
33

44
Any uppercase BUG_* names are modernish shell bug IDs.
55

6+
2022-06-13:
7+
8+
- Trapping a signal that is not a pseudosignal will now cause a virtual
9+
subshell to fork into a real one. This ensures a persistent PID where
10+
other processes can signal the subshell (the PID of a virtual subshell may
11+
change as other commands cause it to fork), and fixes a bug where a signal
12+
could not be correctly or effectively trapped in a subshell if the same
13+
signal was also trapped in the main shell environment.
14+
615
2022-06-12:
716

817
- The POSIX mode now disables zero-padding of seconds in 'time'/'times' output.

‎src/cmd/ksh93/bltins/trap.c‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,13 @@ int b_trap(int argc,char *argv[],Shbltin_t *context)
166166
}
167167
else
168168
{
169+
/*
170+
* Trap or ignore a real signal. A virtual subshell needs to fork in
171+
* order to receive signals correctly and (because other commands
172+
* may cause a virtual subshell to fork) to ensure a persistent PID.
173+
*/
174+
if(sh.subshell && !sh.subshare)
175+
sh_subfork();
169176
if(sig >= sh.st.trapmax)
170177
sh.st.trapmax = sig+1;
171178
arg = sh.st.trapcom[sig];

‎src/cmd/ksh93/include/version.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323

2424
#define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */
2525
#define SH_RELEASE_SVER "1.0.0-beta.2" /* semantic version number: https://semver.org */
26-
#define SH_RELEASE_DATE "2022-06-12" /* must be in this format for $((.sh.version)) */
26+
#define SH_RELEASE_DATE "2022-06-13" /* must be in this format for $((.sh.version)) */
2727
#define SH_RELEASE_CPYR "(c) 2020-2022 Contributors to ksh " SH_RELEASE_FORK
2828

2929
/* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */

‎src/cmd/ksh93/tests/signal.sh‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -273,22 +273,37 @@ PATH=${PATH#:}
273273
if [[ ${SIG[USR1]} ]]
274274
then float s=$SECONDS
275275
exp=SIGUSR1
276+
276277
got=$(LC_ALL=C $SHELL -c '
277278
trap "print SIGUSR1 ; exit 0" USR1
278279
(trap "" USR1 ; exec kill -USR1 $$ & sleep .5)
279280
print done')
280281
[[ $got == "$exp" ]] || err_exit 'subshell ignoring signal does not send signal to parent' \
281-
"(expected '$exp', got '$got')"
282+
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
282283
(( (SECONDS-s) < .4 )) && err_exit 'parent does not wait for child to complete before handling signal'
283284
((s = SECONDS))
284-
exp=SIGUSR1
285+
286+
: >out
287+
trap 'echo SIGUSR1 >out; exit 0' USR1
288+
(trap '' USR1; kill -USR1 $$)
289+
got=$(<out)
290+
[[ $got == "$exp" ]] || err_exit 'subshell ignoring signal does not send signal to parent [simple case]' \
291+
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
292+
285293
got=$(LC_ALL=C $SHELL -c '
286294
trap "print SIGUSR1 ; exit 0" USR1
287295
(trap "exit" USR1 ; exec kill -USR1 $$ & sleep .5)
288296
print done')
289297
[[ $got == "$exp" ]] || err_exit 'subshell catching signal does not send signal to parent' \
290-
"(expected '$exp', got '$got')"
298+
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
291299
(( SECONDS-s < .4 )) && err_exit 'parent completes early'
300+
301+
: >out
302+
trap 'echo SIGUSR1 >out; exit 0' USR1
303+
(trap 'echo wrong' USR1; kill -USR1 $$)
304+
got=$(<out)
305+
[[ $got == "$exp" ]] || err_exit 'subshell catching signal does not send signal to parent [simple case]' \
306+
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
292307
fi
293308
294309
yes() for ((;;)); do print y; done

0 commit comments

Comments
 (0)