[v3] Allow trap to un-ignore SIGINT/SIGQUIT in async subshells

Commit Message

Johannes Altmanninger March 29, 2024, 3:39 p.m. UTC

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.

Builtin [trap] has this requirement:

> Signals that were ignored on entry to a non-interactive shell
> cannot be trapped or reset, although no error need be reported when
> attempting to do so.

Apparently this only applies to signals that were inherited as ignored,
not to the special case of SIGINT/SIGQUIT begin ignored in asynchronous
subshells.

Make it so. This means that either of

	trap - INT; trap - QUIT
	set -i

in a backgrounded subshell will now un-ignore SIGINT and SIGQUIT.

[Signals]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html
[trap]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#trap

{{{ 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"' makes 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/[e-mail]%3E

}}}
---
 src/trap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Herbert Xu April 7, 2024, 11:18 a.m. UTC | #1

On Fri, Mar 29, 2024 at 04:39:01PM +0100, Johannes Altmanninger wrote:
>
> diff --git a/src/trap.c b/src/trap.c
> index cd84814..dbf81ea 100644
> --- a/src/trap.c
> +++ b/src/trap.c
> @@ -272,7 +272,7 @@ ignoresig(int signo)
>  		signal(signo, SIG_IGN);
>  	}
>  	if (!vforked)
> -		sigmode[signo - 1] = S_HARD_IGN;
> +		sigmode[signo - 1] = S_IGN;

This is buggy if sigmode is already S_HARD_IGN.  You can fix
this by moving the if statement inside the previous one.

Please also add a comment stating that sigmode has already been
initialised by setinteractive, as otherwise we may also lose a
hard ignore.

Thanks,

Patch

diff --git a/src/trap.c b/src/trap.c
index cd84814..dbf81ea 100644
--- a/src/trap.c
+++ b/src/trap.c
@@ -272,7 +272,7 @@  ignoresig(int signo)
 		signal(signo, SIG_IGN);
 	}
 	if (!vforked)
-		sigmode[signo - 1] = S_HARD_IGN;
+		sigmode[signo - 1] = S_IGN;
 }