[PATCH v2] Allow trap to un-ignore SIGINT in asynchronous subshells

[Johannes Altmanninger <aclopte@xxxxxxxxx>]

Unlike in Bash or Zsh, this asynchronous job ignores SIGINT, despite
builtin trap explicitly resetting the SIGINT handler.

	dash -c '( trap - INT; sleep inf ) &'

POSIX[*] Section 2.11 on Signals and Error Handling says about
background execution:

> If job control is disabled (see the description of set -m) when
> the shell executes an asynchronous list, the commands in the list
> shall inherit from the shell a signal action of ignored (SIG_IGN)
> for the SIGINT and SIGQUIT signals.  In all other cases, commands
> executed by the shell shall inherit the same signal actions as those
> inherited by the shell from its parent unless a signal action is
> modified by the trap special built-in (see trap)

It is not clear to me from this description whether the trap builtin is
supposed to un-ignore SIGINT and SIGQUIT in the above asynchronous job.
I think yes because processes like "sh -c 'trap ...'"  can already
do that, so why treat subshells differently.  The Bash maintainer
seems to agree when responding to a related bug report[**]
although that one is not about subshells specifically.

> The issue is that the processes in this list have to ignore SIGINT
> [...] but they have to be allowed to use trap to change the signal
> dispositions (POSIX interp 751)

This issue exists because we "hard-ignore" (meaning ignore
permanently) SIGINT and SIGQUIT in backgrounded subshells; I'm
not sure why.  Git blame is not helpful since this existed since
the initial Git import. I failed to find a test suite; no luck at
http://www.opengroup.org/testing/testsuites/vscpcts2003.htm where I
get SSL errors.

Since I don't see any reason for hard-ignore logic, remove it
altogether.  This means that either of

	trap - INT; trap - QUIT
	set -i

in a backgrounded subshell will now un-ignore SIGINT and SIGQUIT.
I did not find other behavior changes but maybe there is a good reason
for S_HARD_IGN?

[*]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html
[**]: https://lists.gnu.org/archive/html/bug-bash/2023-01/msg00050.html

{{{ Test cases:

shell=src/dash

set -e

SubshellWith() {
	parent_pid=$(setsid "$shell" -c "( $1; sleep 99 ) </dev/null >/dev/null 2>&1 & echo \$\$")
	sleep 1
	subshell_pid=$(ps -o pid= -$parent_pid | tail -n 1)
}

trap 'kill -TERM -$parent_pid 2>/dev//null ||:' EXIT # Tear down after a failure.

echo Scenario 0: '"set -i"' maks a subshell un-ignore SIGINT.
SubshellWith 'set -i'
kill -INT $subshell_pid
! ps -p $subshell_pid | grep sleep || exit 1
kill -TERM -$parent_pid 2>/dev//null ||: # Tear down.

echo Scenario 1: resetting SIGINT handler.
SubshellWith 'trap - INT'
kill -INT -$parent_pid # kill the whole process group since that's the my use case
! ps -p $subshell_pid | grep sleep || exit 1
kill -TERM -$parent_pid 2>/dev//null ||: # Tear down.

echo Scenario 2: ignoring SIGINT.
SubshellWith 'trap "" INT'
kill -INT $subshell_pid
ps -p $subshell_pid | grep sleep || exit 1
kill -TERM -$parent_pid 2>/dev//null ||: # Tear down.

}}}

{{{ Backstory/motivation:

The Kakoune[1] editor likes to run noninteractive shell commands that
boil down to

	mkfifo /tmp/fifo
	(
		trap - INT
		make
	) >/tmp/fifo 2>&1 &

On Control-C, the editor sends SIGINT to its process group,
which should terminate the subshell running make[2].

We experimented with sending SIGTERM instead but found issues,
specifically if the editor is invoked (without exec) from a wrapper
script, sending SIGTERM to the whole process group would kill the
wrapper script, which in turn makes it send SIGTERM to the editor,
which then terminates.

[1]: https://kakoune.org/
[2]: https://lists.sr.ht/~mawww/kakoune/%3C20240307135831.1967826-3-aclopte@xxxxxxxxx%3E

}}}
---
 src/trap.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/src/trap.c b/src/trap.c
index cd84814..d80fdde 100644
--- a/src/trap.c
+++ b/src/trap.c
@@ -55,14 +55,12 @@
 /*
  * Sigmode records the current value of the signal handlers for the various
  * modes.  A value of zero means that the current handler is not known.
- * S_HARD_IGN indicates that the signal was ignored on entry to the shell,
  */
 
 #define S_DFL 1			/* default signal handling (SIG_DFL) */
 #define S_CATCH 2		/* signal is caught */
 #define S_IGN 3			/* signal is ignored (SIG_IGN) */
-#define S_HARD_IGN 4		/* signal is ignored permenantly */
-#define S_RESET 5		/* temporary - to reset a hard ignored sig */
+#define S_RESET 4		/* temporary - to reset an ignored sig */
 
 
 /* trap handler commands */
@@ -233,16 +231,12 @@ setsignal(int signo)
 			return;
 		}
 		if (act.sa_handler == SIG_IGN) {
-			if (mflag && (signo == SIGTSTP ||
-			     signo == SIGTTIN || signo == SIGTTOU)) {
-				tsig = S_IGN;	/* don't hard ignore these */
-			} else
-				tsig = S_HARD_IGN;
+			tsig = S_IGN;
 		} else {
 			tsig = S_RESET;	/* force to be set */
 		}
 	}
-	if (tsig == S_HARD_IGN || tsig == action)
+	if (tsig == action)
 		return;
 	switch (action) {
 	case S_CATCH:
@@ -268,11 +262,11 @@ setsignal(int signo)
 void
 ignoresig(int signo)
 {
-	if (sigmode[signo - 1] != S_IGN && sigmode[signo - 1] != S_HARD_IGN) {
+	if (sigmode[signo - 1] != S_IGN) {
 		signal(signo, SIG_IGN);
 	}
 	if (!vforked)
-		sigmode[signo - 1] = S_HARD_IGN;
+		sigmode[signo - 1] = S_IGN;
 }
 
 
-- 
2.44.0.368.gc75fd8d815