1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-02-15 04:32:24 +00:00

Fix SIGINT handling for external commands run from scripts

Reproducer:

  $ ksh -c 'bash -c '\''kill -s INT $$'\''; echo "$?, continuing"'

Expected result: output "258, continuing"; exit status 0.

Actual result: no output; exit status 258. The child process sent
SIGINT only to itself and not to the process group, so the parent
script was wrongly interrupted.

Every shell except ksh93 produces the expected result. ksh93 also
gave the expected result before version 2008-01-31 93s+, which
introduced the code below.

Analysis: The problem is in these lines of code in xec.c,
sh_exec(), TFORK case, parent branch of fork:

1649:	if(!sh_isstate(SH_MONITOR))
1650:	{
1651:		if(!(sh.sigflag[SIGINT]&(SH_SIGFAULT|SH_SIGOFF)))
1652:			sh_sigtrap(SIGINT);
1653:		sh.trapnote |= SH_SIGIGNORE;
1654:	}
[...pipe and I/O handling, wait for command to finish...]
1667:	if(!sh_isstate(SH_MONITOR))
1668:	{
1669:		sh.trapnote &= ~SH_SIGIGNORE;
1700:		if(sh.exitval == (SH_EXITSIG|SIGINT))
1701:			kill(sh.current_pid,SIGINT);
1702:	}

When a user presses Ctrl+C, SIGINT is sent to the entire process
group. If job control is fully off (i.e., !sh_isstate(SH_MONITOR)),
then the process group includes the parent script. Therefore, in a
script such as

  $ ksh -c 'bash -c '\''read x'\''; echo "$?, continuing"'

when the user presses Ctrl+C while bash waits for 'read x' input,
the parent ksh script should be interrupted as well.

Now, the code above ignores SIGINT while bash is running. (This is
done using special-casing in sh_fault() to handle that SH_SIGIGNORE
flag for SIGINT.) So, when Ctrl+C interrupts the process group, the
parent script is not getting interrupted as it should.

To compensate for that, the code then detects, using sh.exitval
(the child process' exit status), whether the child process was
killed by SIGINT. If so, it simply assumes that the signal was
meant for the process group including the parent script, so it
reissues SIGINT to itself after unignoring it.

But, as we can see from the broken reproducer above, that
assumption is not valid. Scripts are perfectly free to send SIGINT
to themselves only, and that must work as expected.

src/cmd/ksh93/sh/xec.c: sh_exec(): TFORK: parent branch:
- Instead of ignoring SIGINT, sigblock() it, which delays handling
  the signal until sigrelease(). (Note that these are macros
  defined in src/cmd/ksh93/features/sigfeatures according to OS
  capabilities.)
- This makes reissuing SIGINT redundant, so delete that, which
  fixes the bug.

src/cmd/ksh93/sh/fault.c:
- Nothing now sets the SH_SIGIGNORE flag in sh.trapnote, so remove
  special-casing added in 2008-01-31 93s+.
This commit is contained in:
Martijn Dekker 2022-02-01 03:11:10 +00:00
parent 9a5af738ef
commit d650c73e55
4 changed files with 25 additions and 12 deletions

4
NEWS
View file

@ -22,6 +22,10 @@ Any uppercase BUG_* names are modernish shell bug IDs.
- In emacs mode, Insert escapes the next character.
- In vi mode, Insert will switch the editor to insert mode (like in vim).
- Fixed a bug in SIGINT handling: if a script ran an external command that
interrupted itself (and only itself, not the process group) with SIGINT,
the script that ran the command was also interrupted.
2022-01-28:
- Fixed longstanding job control breakage that occurred on the interactive

View file

@ -113,8 +113,6 @@ void sh_fault(register int sig)
flag = sh.sigflag[sig]&~SH_SIGOFF;
if(!trap)
{
if(sig==SIGINT && (sh.trapnote&SH_SIGIGNORE))
return;
if(flag&SH_SIGIGNORE)
{
if(sh.subshell)
@ -386,7 +384,7 @@ void sh_chktrap(void)
{
register int sig=sh.st.trapmax;
register char *trap;
if(!(sh.trapnote&~SH_SIGIGNORE))
if(!sh.trapnote)
sig=0;
sh.trapnote &= ~SH_SIGTRAP;
/* execute errexit trap first */

View file

@ -1650,7 +1650,7 @@ int sh_exec(register const Shnode_t *t, int flags)
{
if(!(sh.sigflag[SIGINT]&(SH_SIGFAULT|SH_SIGOFF)))
sh_sigtrap(SIGINT);
sh.trapnote |= SH_SIGIGNORE;
sigblock(SIGINT);
}
if(sh.pipepid)
sh.pipepid = parent;
@ -1665,11 +1665,7 @@ int sh_exec(register const Shnode_t *t, int flags)
if(usepipe && tsetio && subdup && unpipe)
sh_iounpipe();
if(!sh_isstate(SH_MONITOR))
{
sh.trapnote &= ~SH_SIGIGNORE;
if(sh.exitval == (SH_EXITSIG|SIGINT))
kill(sh.current_pid,SIGINT);
}
sigrelease(SIGINT);
}
if(type&FAMP)
{

View file

@ -76,6 +76,8 @@ expect=01got_child23
[[ $actual == "$expect" ]] || err_exit 'SIGCHLD not working' \
"(expected $(printf %q "$expect"), got $(printf %q "$actual"))"
# ======
# begin standalone SIGINT test generation
cat > tst <<'!'
@ -243,14 +245,14 @@ chmod +x tst tst-?
# end standalone test generation
export PATH=$PATH:
PATH=:$PATH
typeset -A expected
expected[---]="3-intr"
expected[--d]="3-intr"
expected[-t-]="3-intr 2-intr 1-intr 1-0258"
expected[-td]="3-intr 2-intr 1-intr 1-0258"
expected[x--]="3-intr 2-intr 1-0000"
expected[x-d]="3-intr 2-intr 1-0000"
expected[x--]="3-intr 2-intr"
expected[x-d]="3-intr 2-intr"
expected[xt-]="3-intr 2-intr 1-intr 1-0000"
expected[xtd]="3-intr 2-intr 1-intr 1-0000"
expected[z--]="3-intr 2-intr 1-0000"
@ -263,6 +265,10 @@ tst $SHELL > tst.got
while read ops out
do [[ $out == ${expected[$ops]} ]] || err_exit "interrupt $ops test failed -- expected '${expected[$ops]}', got '$out'"
done < tst.got
unset expected
PATH=${PATH#:}
# ======
if [[ ${SIG[USR1]} ]]
then float s=$SECONDS
@ -553,5 +559,14 @@ got=$( set +x; { "$SHELL" bar; } 2>&1 )
(( (e=$?)==0 )) && [[ $got == "$exp" ]] || err_exit "segfaulting child process:" \
"(expected status 0 and $(printf %q "$exp"), got status $e and $(printf %q "$got"))"
# ======
# A script that SIGINTs only itself (not the process group) should not cause the parent script to be interrupted
trap '' INT # workaround for old ksh -- ignore SIGINT or the entire test suite gets interrupted
exp='258, continuing'
got=$("$SHELL" -c 'trap + INT; "$SHELL" -c '\''kill -s INT $$'\''; echo "$?, continuing"')
((!(e = $?))) && [[ $got == "$exp" ]] || err_exit "child process interrupting itself interrupts parent" \
"(got status $e$( ((e>128)) && print -n /SIG && kill -l "$e"), $(printf %q "$got"))"
trap - INT
# ======
exit $((Errors<125?Errors:125))