Re: Bug with trap?

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

Peter Stephenson wrote:
> bash (4.0.35) does what zsh does before this patch, only executes the
> trap on an explicit exit.

(This is getting hairy so I've moved it to zsh-workers.)

It's occurred to me that I'm falling for the old chestnut that bash,
unlike zsh, excutes the left (not the right) of a pipeline in the current
shell---which, of course, doesn't exit implicitly after the pipeline.
This suggests there's some ambiguity in what should happen here.

  ( trap 'echo exiting' EXIT; ) | cat

does print a message in bash but not zsh, even after the previous patch
(another bug, fixed below), but

  true | { trap 'echo exiting' EXIT; } | cat

doesn't in bash though does in in zsh after the patch, which I can't
quite rationalise, but I'm certainly not sure it's wrong.

There's obviously a lot of subtlety here.

In any case I got the previous patch wrong; it's actually too eager to
execute traps for anything run in the left hand side of the pipeline.
It should only be for a list of shell commands that exits.

Other cases I missed before include anything along the lines of:

echo $( trap 'echo exiting' EXIT)

or similar substitutions.  That's quite clearly a subshell entering to
the same extent a ( ... ) is.

It looks like there's a simple fix for a whole heap of these: detect
that we're about to exit at the end of execlist(), but make sure the
trap is then reset (since we're about to exit we obviously don't need it
again).  Then the flag I added to execcmd() comes into its own.

One case I'm not sure about is the final effect here:

% trap 'echo exiting' EXIT
% ( true; ) | cat
% { true; } | cat
exiting

That's explicit in the code---see the ESUB_KEEPTRAP flag near the top of
the second hunk below.  But is it correct, since we not only forked, but
always fork on the left hand side of a pipe?  Surely we're being too
clever for our own good here?

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.177
diff -p -u -r1.177 exec.c
--- Src/exec.c	24 Feb 2010 21:38:10 -0000	1.177
+++ Src/exec.c	4 May 2010 21:23:18 -0000
@@ -1299,6 +1299,12 @@ sublist_done:
     lineno = oldlineno;
     if (dont_change_job)
 	thisjob = cj;
+
+    if (exiting && sigtrapped[SIGEXIT]) {
+	dotrap(SIGEXIT);
+	/* Make sure this doesn't get executed again. */
+	sigtrapped[SIGEXIT] = 0;
+    }
 }
 
 /* Execute a pipeline.                                                *
@@ -1631,7 +1637,7 @@ execpline2(Estate state, wordcode pcode,
 		entersubsh(((how & Z_ASYNC) ? ESUB_ASYNC : 0)
 			   | ESUB_PGRP | ESUB_KEEPTRAP);
 		close(synch[1]);
-		execcmd(state, input, pipes[1], how, 0);
+		execcmd(state, input, pipes[1], how, 1);
 		_exit(lastval);
 	    }
 	} else {
Index: Test/C03traps.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/C03traps.ztst,v
retrieving revision 1.17
diff -p -u -r1.17 C03traps.ztst
--- Test/C03traps.ztst	31 Aug 2008 19:50:49 -0000	1.17
+++ Test/C03traps.ztst	4 May 2010 21:23:18 -0000
@@ -350,6 +350,26 @@
 >trap
 >Working 0
 
+   { trap 'echo This subshell is exiting' EXIT; } | cat
+0: EXIT trap set in current shell at left of pipeline
+>This subshell is exiting
+
+   ( trap 'echo This subshell is also exiting' EXIT; ) | cat
+0: EXIT trap set in subshell at left of pipeline
+>This subshell is also exiting
+
+   ( trap 'echo Should only appear once at the end' EXIT
+     ( : trap reset here ) | cat
+     : trap not reset but not part of shell command list | cat
+     echo nothing after this should appear $( : trap reset here too)
+   )
+0: EXIT trap set in subshell reset in subsubshell
+>nothing after this should appear
+>Should only appear once at the end
+
+   echo $( trap 'echo command substitution exited' EXIT )
+0: EXIT trap set in command substitution
+>command substitution exited
 
 %clean
 

-- 
Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/