Force fork if any trap is set, not just on EXIT.

PATCH In review Jilles Tjoelker 2 participants 2 messages May 14, 2010 – May 27, 2010

Share & tools

Jilles Tjoelker

author

2010-05-14 21:50 UTC #0 lore raw

PATCH 1/1 Force fork if any trap is set, not just on EXIT.

In some cases the shell executes a subshell or an external command in the current process. This is not done if a trap on EXIT has been set, so that that trap can execute after the subshell or external command has finished. Extend that check to all traps. (A trap is "set" if a non-empty command string has been attached to it.)

Improve encapsulation by exporting an accessor function for this and making the trap array static again.

This is much like FreeBSD SVN r194127, enhanced to apply to subshells also (see FreeBSD SVN r194774).

Example:
dash -c '{ trap "echo moo" TERM; sleep 3; }& sleep 1; kill $!;wait'
This should print "moo" after 3 seconds.

Example:
dash -c '{ trap "echo moo" TERM; (sleep 3) }& sleep 1; kill $!;wait'
The same.

Example:
dash -c '{ trap "echo moo" TERM; sleep 3; :; }& sleep 1; kill $!;wait'
This works correctly even without this patch.

Signed-off-by Jilles Tjoelker <[email protected]>

---
 src/eval.c |    4 ++--
 src/trap.c |   16 +++++++++++++++-
 src/trap.h |    2 +-
 3 files changed, 18 insertions(+), 4 deletions(-)

src/eval.c +2 -2

index 62d9d5d..e7c95cc 100644
@@ -448,7 +448,7 @@ evalsubshell(union node *n, int flags)
 	int status;
 
 	expredir(n->nredir.redirect);
-	if (!backgnd && flags & EV_EXIT && !trap[0])
+	if (!backgnd && flags & EV_EXIT && !have_traps())
 		goto nofork;
 	INTOFF;
 	jp = makejob(n, 1);

@@ -829,7 +829,7 @@ bail:
 	switch (cmdentry.cmdtype) {
 	default:
 		/* Fork off a child process if necessary. */
-		if (!(flags & EV_EXIT) || trap[0]) {
+		if (!(flags & EV_EXIT) || have_traps()) {
 			INTOFF;
 			jp = makejob(cmd, 1);
 			if (forkshell(jp, cmd, FORK_FG) != 0) {

src/trap.c +15 -1

index 3f93c46..439b8d2 100644
@@ -69,7 +69,7 @@
 
 
 /* trap handler commands */
-char *trap[NSIG];
+static char *trap[NSIG];
 /* current value of signal */
 char sigmode[NSIG - 1];
 /* indicates specified signal received */

@@ -163,6 +163,20 @@ clear_traps(void)
 }
 
 
+/*
+ * Check if we have any traps enabled.
+ */
+int
+have_traps(void)
+{
+	char *volatile *tp;
+
+	for (tp = trap ; tp <= &trap[NSIG - 1] ; tp++) {
+		if (*tp && **tp)	/* trap not NULL or SIG_IGN */
+			return 1;
+	}
+	return 0;
+}
 
 /*
  * Set the signal handler for the specified signal.  The routine figures

src/trap.h +1 -1

index f19a66b..4e897a9 100644
@@ -36,12 +36,12 @@
 
 #include <signal.h>
 
-extern char *trap[];
 extern char sigmode[];
 extern volatile sig_atomic_t pendingsigs;
 
 int trapcmd(int, char **);
 void clear_traps(void);
+int have_traps(void);
 void setsignal(int);
 void ignoresig(int);
 void onsig(int);

Signature · 1 line

1.7.1

Herbert Xu

2010-05-27 08:09 UTC #1 lore raw

Quoted 20 lines · Jilles Tjoelker

In some cases the shell executes a subshell or an external command in the current process. This is not done if a trap on EXIT has been set, so that that trap can execute after the subshell or external command has finished. Extend that check to all traps. (A trap is "set" if a non-empty command string has been attached to it.)
Improve encapsulation by exporting an accessor function for this and making the trap array static again.
This is much like FreeBSD SVN r194127, enhanced to apply to subshells also (see FreeBSD SVN r194774).
Example:
dash -c '{ trap "echo moo" TERM; sleep 3; }& sleep 1; kill $!;wait'
This should print "moo" after 3 seconds.
Example:
dash -c '{ trap "echo moo" TERM; (sleep 3) }& sleep 1; kill $!;wait'
The same.

Quoted 5 lines

Example:
dash -c '{ trap "echo moo" TERM; sleep 3; :; }& sleep 1; kill $!;wait'
This works correctly even without this patch.
Signed-off-by Jilles Tjoelker <[email protected]>

I've applied it with some changes. In particuar, I've made the have_traps code done using a count kept up-to-date by trapcmd and clear_traps. Let me know if you see any problems with this.

ChangeLog +4 -0

index 650899a..969d6be 100644
@@ -1,3 +1,7 @@
+2010-05-27  Jilles Tjoelker <[email protected]>
+
+	* Force fork if any trap is set, not just on EXIT.
+
 2010-05-27  Herbert Xu <[email protected]>
 
 	* Fix poplocalvar on abnormal exit from function.

src/eval.c +2 -2

index d5c1e6c..439f881 100644
@@ -449,7 +449,7 @@ evalsubshell(union node *n, int flags)
 	int status;
 
 	expredir(n->nredir.redirect);
-	if (!backgnd && flags & EV_EXIT && !trap[0])
+	if (!backgnd && flags & EV_EXIT && !have_traps())
 		goto nofork;
 	INTOFF;
 	jp = makejob(n, 1);

@@ -836,7 +836,7 @@ bail:
 	switch (cmdentry.cmdtype) {
 	default:
 		/* Fork off a child process if necessary. */
-		if (!(flags & EV_EXIT) || trap[0]) {
+		if (!(flags & EV_EXIT) || have_traps()) {
 			INTOFF;
 			jp = makejob(cmd, 1);
 			if (forkshell(jp, cmd, FORK_FG) != 0) {

src/trap.c +14 -5

index 3f93c46..ba32da6 100644
@@ -69,7 +69,9 @@
 
 
 /* trap handler commands */
-char *trap[NSIG];
+static char *trap[NSIG];
+/* number of non-null traps */
+int trapcnt;
 /* current value of signal */
 char sigmode[NSIG - 1];
 /* indicates specified signal received */

@@ -125,11 +127,17 @@ trapcmd(int argc, char **argv)
 		if (action) {
 			if (action[0] == '-' && action[1] == '\0')
 				action = NULL;
-			else
+			else {
+				if (*action)
+					trapcnt++;
 				action = savestr(action);
+			}
 		}
-		if (trap[signo])
+		if (trap[signo]) {
+			if (*trap[signo])
+				trapcnt--;
 			ckfree(trap[signo]);
+		}
 		trap[signo] = action;
 		if (signo != 0)
 			setsignal(signo);

@@ -150,16 +158,17 @@ clear_traps(void)
 {
 	char **tp;
 
+	INTOFF;
 	for (tp = trap ; tp < &trap[NSIG] ; tp++) {
 		if (*tp && **tp) {	/* trap not NULL or SIG_IGN */
-			INTOFF;
 			ckfree(*tp);
 			*tp = NULL;
 			if (tp != &trap[0])
 				setsignal(tp - trap);
-			INTON;
 		}
 	}
+	trapcnt = 0;
+	INTON;
 }

src/trap.h +6 -1

index f19a66b..50fc3ed 100644
@@ -36,7 +36,7 @@
 
 #include <signal.h>
 
-extern char *trap[];
+extern int trapcnt;
 extern char sigmode[];
 extern volatile sig_atomic_t pendingsigs;

@@ -49,3 +49,8 @@ int dotrap(void);
 void setinteractive(int);
 void exitshell(void) __attribute__((__noreturn__));
 int decode_signal(const char *, int);
+
+static inline int have_traps(void)
+{
+	return trapcnt;
+}

Thanks,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt