[RFC PATCH] Allow trap to override permanently-ignored signals in background shells

From: Johannes Altmanninger <aclopte@gmail.com>
To: dash@vger.kernel.org
Cc: Johannes Altmanninger <aclopte@gmail.com>
Subject: [RFC PATCH] Allow trap to override permanently-ignored signals in background shells
Date: Fri,  8 Mar 2024 12:01:55 +0100	[thread overview]
Message-ID: <20240308110158.2459219-1-aclopte@gmail.com> (raw)

TL;DR: I think this job should exit on Control-C.

	( trap - INT; sleep inf ) &

With shell=bash or shell=zsh this works with below script.
Meanwhile, shell=dash fails

	shell=dash
	set -e
	n=123
	pkill -f "sleep.$n" ||:
	pid=$(setsid "$shell" -c "( trap - INT; sleep $n ) </dev/null >/dev/null 2>&1 & echo \$\$")
	sleep 1
	kill -INT -$pid
	ps aux | grep "[s]leep.$n"

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.

and continues:

> 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 whether the trap builtin is supposed to override
the ignore. Intuitively, I'd say yes, and the Bash maintainer seems
to agree responding to a related bug report about Bash functions[**]

> 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)

Attached patch works for me though it's barely thought-through or
tested. Happy to take it to completion.

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

{{{

Backstory:

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

	mkfifo /tmp/fifo
	(
		sleep inf # in practice "make"
	) >/tmp/fifo 2>&1 &

On Control-C, the editor traditionally sends SIGINT to the process
group. Bash documentation says[2]:

> When job control is not in effect, asynchronous commands ignore
> SIGINT and SIGQUIT in addition to these inherited handlers.

This means that the "sleep inf" process ignores SIGINT.
We have worked around this fact by sending SIGTERM instead[3].

If the forked process sets a SIGINT handler of any form, for example
by using "signal(SIGINT, SIG_DFL)", it will no longer ignore SIGINT.

It seems reasonable to give (sub)shells the same powers, via the
"trap" builtin. This would allow me to change the above subshell to

	(
		trap - INT
		sleep inf
	) >/tmp/fifo 2>&1 &

to have it be terminated by SIGINT.

In general, I wonder if SIGINT is only for actual shells, and SIGTERM
is the signal to use in our situation.

[1]: https://kakoune.org/
[2]: https://www.gnu.org/software/bash/manual/html_node/Signals.html
[3]: https://lists.sr.ht/~mawww/kakoune/%3C20240307135831.1967826-3-aclopte@gmail.com%3E

}}}
---
 src/trap.c | 12 +++++++++---
 src/trap.h |  1 +
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/trap.c b/src/trap.c
index cd84814..a6778e8 100644
--- a/src/trap.c
+++ b/src/trap.c
@@ -159,7 +159,7 @@ trapcmd(int argc, char **argv)
 		}
 		trap[signo] = action;
 		if (signo != 0)
-			setsignal(signo);
+			setsignal_maybe_user(signo, 1);
 		INTON;
 		ap++;
 	}
@@ -175,6 +175,12 @@ trapcmd(int argc, char **argv)
 
 void
 setsignal(int signo)
+{
+    setsignal_maybe_user(signo, 0);
+}
+
+void
+setsignal_maybe_user(int signo, int user)
 {
 	int action;
 	int lvforked;
@@ -234,7 +240,7 @@ setsignal(int signo)
 		}
 		if (act.sa_handler == SIG_IGN) {
 			if (mflag && (signo == SIGTSTP ||
-			     signo == SIGTTIN || signo == SIGTTOU)) {
+			              signo == SIGTTIN || signo == SIGTTOU)) {
 				tsig = S_IGN;	/* don't hard ignore these */
 			} else
 				tsig = S_HARD_IGN;
@@ -242,7 +248,7 @@ setsignal(int signo)
 			tsig = S_RESET;	/* force to be set */
 		}
 	}
-	if (tsig == S_HARD_IGN || tsig == action)
+	if ((!user && tsig == S_HARD_IGN) || tsig == action)
 		return;
 	switch (action) {
 	case S_CATCH:
diff --git a/src/trap.h b/src/trap.h
index beaf660..595992c 100644
--- a/src/trap.h
+++ b/src/trap.h
@@ -43,6 +43,7 @@ extern volatile sig_atomic_t gotsigchld;
 
 int trapcmd(int, char **);
 void setsignal(int);
+void setsignal_maybe_user(int, int);
 void ignoresig(int);
 void onsig(int);
 void dotrap(void);
-- 
2.44.0.rc1.17.g3e0d3cd5c7

