Interactive ksh terminates on ^C when SIGINT is ignored on entry

[McDutchie]

Symptom (at least on macOS):

$ (trap '' INT; ENV=/./dev/null PS1='childshell$ ' ksh)
childshell$ (press Ctrl+C)
$ 

The child shell exited (with status 0) upon pressing Ctrl+C.

This bug was introduced by a Red Hat patch that I applied in 7e5fd3e. We need to figure out how to fix it without reintroducing the bug that it fixed.

[McDutchie]

I can confirm that this bug is present in the Red Hat CentOS's patched stock version of ksh 93u+ 2012-08-01.

[McDutchie]

The following change from that commit introduced the bug:

--- a/src/cmd/ksh93/edit/edit.c
+++ b/src/cmd/ksh93/edit/edit.c
@@ -1056,7 +1056,7 @@ int ed_getchar(register Edit_t *ep,int mode)
 		{
 			if(mode<=0 && -c == ep->e_intr)
 			{
-				sh_fault(SIGINT);
+				killpg(getpgrp(),SIGINT);
 				siglongjmp(ep->e_env, UINTR);
 			}
 			if(mode<=0 && ep->sh->st.trap[SH_KEYTRAP])

Reverting it fixes this bug, but reintroduces the original bug, rhbz#960034 – unfortunately one of those private Red Hat bugs, but I was granted access for backporting RedHat patches. The original bug reintroduced by reverting this change is this one:

With the monitor option off (set +m/set +o monitor), background processes do not get their own process group, but share the shell's process group. When you press Ctrl+C, any background processes that are in the same process group as the current shell should be terminated with SIGINT. So, if youset +m, run something like xterm &, and then press Ctrl+C, your xterm should disappear. ksh did not do this before this Red Hat change. That is what the killpg call is for.

Now, for some reason that I don't understand yet, if SIGINT was ignored upon invoking ksh, then issuing SIGINT to ksh itself will kill an interactive ksh. The killpg call issues SIGINT to the entire process group, including ksh itself.

So one way to fix it would be to issue SIGINT to the entire process group except ksh itself; then reinstate the old sh_fault call for handling SIGINT in the main shell.

Actually excluding the current process from a killpg call does not seem to be feasible, but blocking SIGINT in the main process just for the killpg call seems to be good enough. (Note the sigblock and sigrelease macros are defined here in sigfeatures.)

Patch:

--- a/src/cmd/ksh93/edit/edit.c
+++ b/src/cmd/ksh93/edit/edit.c
@@ -1120,7 +1120,12 @@ int ed_getchar(register Edit_t *ep,int mode)
 		{
 			if(mode<=0 && -c == ep->e_intr)
 			{
+				/* avoid potentially killing main shell: issue SIGINT to process group except self */
+				sigblock(SIGINT);
 				killpg(getpgrp(),SIGINT);
+				sigrelease(SIGINT);
+				/* now handle SIGINT in current process */
+				sh_fault(SIGINT);
 				siglongjmp(ep->e_env, UINTR);
 			}
 			if(mode<=0 && ep->sh->st.trap[SH_KEYTRAP])

[McDutchie]

Actually, this seems to be a more efficient way to accomplish the same thing. Somehow SIGINT got reset to default, so just restore the expected sh_fault handler.

--- a/src/cmd/ksh93/edit/edit.c
+++ b/src/cmd/ksh93/edit/edit.c
@@ -1120,6 +1120,7 @@ int ed_getchar(register Edit_t *ep,int mode)
 		{
 			if(mode<=0 && -c == ep->e_intr)
 			{
+				signal(SIGINT,sh_fault);
 				killpg(getpgrp(),SIGINT);
 				siglongjmp(ep->e_env, UINTR);
 			}

[McDutchie]

Even with either patch, there is still a bug left, and it is not in the original ksh:

$ (trap '' INT; ENV=/./dev/null PS1=child$\  arch/*/bin/ksh -o emacs )
child$ trap 'echo test' INT
child$ 
$

The child shell still exits (with status 0) after trapping SIGINT.

On my system, this bug was introduced by commit ff385e5. A ksh compiled immediately before that commit does not have the bug. I find that mysterious, as that commit seems unrelated to any signal handling.

I'm in over my head here and would appreciate some more eyes on this. Ping @JohnoKing @hyenias @hvdijk

[JohnoKing]

The child shell still exits (with status 0) after trapping SIGINT.

I have reproduced this bug on Linux in ksh93u+m, ksh93u+, ksh93v- and ksh2020 (commit ff385e5 doesn't introduce the bug on my system).

[hyenias]

I also tried out CTRL+C within a child shell and all of my systems that I have tested exit back to the parent shell (FreeBSD, Debian, Ubuntu) with a recent ksh93u+m or ksh93v-. Default system ksh93u+ on macOS and Ubuntu 18.04 returned a new editor line after I issued a CTRL+C.

I tried the following for the subshells and both performed in the same manner:

( trap '' INT; PS1=child$\  ksh +E --emacs )
( ulimit -t unlimited; trap '' INT; PS1=child$\  ksh +E --emacs )

[JohnoKing]

I have been experimenting with this issue and have found another reproducer and a related issue. Trapping SIGINT in the main shell with trap '' INT will cause the shell to exit after an interrupt:

# This reproducer works as expected in ksh93t+ 2010-03-05, but breaks from ksh93t+ 2010-06-14 onwards
$ exec  ksh -o emacs
$ trap '' INT
$ <Ctrl+C>  # This causes ksh93t+ 2010-06-14 and ksh93u+m to exit

Additionally, modifying the trap to run a dummy command causes the prompt to act strangely (this behavior is similar to #202):

# This bug exists in all versions of ksh I've tested, including ksh93n- 2002-06-28
$ exec ksh -o emacs
$ trap 'true' INT
$ <Ctrl+C * 10>
$ $ $ $ $ $ $ $ $ $ $   # An extra '$ ' is added for each Ctrl+C

[McDutchie]

sh_main() calls exfile() and then sh_done(), and that exfile() call is the entire shell session, so the fix is to stop exfile() from returning.

I've experimentally determined that Ctrl+C with SIGINT ignored causes an sfio error code of 256. I can't find where that happens, but all the same, the following patch fixes all the above reproducers for me.

edit: actually, that 256 is probably SH_EXITSIG (defined as 0400 == 256), which would make some sort of sense: it's the signal exit bit without any signal. Patch updated to use that.

--- a/src/cmd/ksh93/sh/main.c
+++ b/src/cmd/ksh93/sh/main.c
@@ -536,6 +536,12 @@ static void	exfile(register Shell_t *shp, register Sfio_t *iop,register int fno)
 		if(tdone || !sfreserve(iop,0,0))
 		{
 		eof_or_error:
+			if(sh_isstate(SH_INTERACTIVE) && sferror(iop)==SH_EXITSIG)
+			{
+				/* Ctrl+C with SIGINT ignored */
+				sfputc(sfstderr,'\n');
+				continue;
+			}
 			if(sh_isstate(SH_INTERACTIVE) && !sferror(iop)) 
 			{
 				if(--maxtry>0 && sh_isoption(SH_IGNOREEOF) &&