exit trap and list pipelines

exit trap and list pipelines

[Anthony Heading]

Hi,

Printing '6' here seems wrong?  I think it didn't some years back.

% zsh -c 'trap "echo hello" EXIT; { :; } | wc -c'
6
hello

% bash -c 'trap "echo hello" EXIT; { :; } | wc -c'
0
hello

Anthony

Re: exit trap and list pipelines

[Bart Schaefer]

On Nov 9,  9:36pm, Anthony Heading wrote:
}
} Printing '6' here seems wrong?  I think it didn't some years back.
} 
} % zsh -c 'trap "echo hello" EXIT; { :; } | wc -c'
} 6
} hello

zsh-2.6 and zsh-3.0 yeild 0, zsh-4.3.17 prints 6.

The change occured because, where zsh formerly would note that the
right side was an external command and "exec" it (thereby disabling
the exit trap), newer zsh notices that there is a trap and does
another fork so that the trap can execute after wc finishes.  This
means that both sides of the pipe run the trap, whereas before
(and in bash) only one side does so.

However, in the older zsh case, there were circumstances in which
the trap did not execute *at all*.

commit 261193a5b7da4ba36ca146424b000aca27c69235
Author: Peter Stephenson <pws@users.sourceforge.net>
Date:   Fri Mar 30 16:51:54 2001 +0000

    Fix problem with traps not runing if shell exec'd final command

(workers/13851)

Re: exit trap and list pipelines

[Peter Stephenson]

On Wed, 9 Nov 2016 21:19:57 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Nov 9,  9:36pm, Anthony Heading wrote:
> }
> } Printing '6' here seems wrong?  I think it didn't some years back.
> } 
> } % zsh -c 'trap "echo hello" EXIT; { :; } | wc -c'
> } 6
> } hello
> 
> zsh-2.6 and zsh-3.0 yeild 0, zsh-4.3.17 prints 6.
> 
> The change occured because, where zsh formerly would note that the
> right side was an external command and "exec" it (thereby disabling
> the exit trap), newer zsh notices that there is a trap and does
> another fork so that the trap can execute after wc finishes.  This
> means that both sides of the pipe run the trap, whereas before
> (and in bash) only one side does so.

I think, therefore, we can just suppress it on the left hand side of the
pipe.  The case where the LHS is a shell construct is special, so I
suppose it just needs extra treatment.

I've added a test for the other case but it was already working.

This shouldn't affect EXIT traps in functions as you need to set them
inside the function in question.

The fact I've replaced wc on the right with a shell construct in the
test doesn't affect this bug, because we've already forked for the LHS,
but it's arguably a different case to having an external command on the
right.  In practice I don't think we're sensitive to the difference.

pws

diff --git a/Src/exec.c b/Src/exec.c
index c0ed2c4..a01a633 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1888,6 +1888,10 @@ execpline2(Estate state, wordcode pcode,
 		entersubsh(((how & Z_ASYNC) ? ESUB_ASYNC : 0)
 			   | ESUB_PGRP | ESUB_KEEPTRAP);
 		close(synch[1]);
+		if (sigtrapped[SIGEXIT])
+		{
+		    unsettrap(SIGEXIT);
+		}
 		execcmd_exec(state, &eparams, input, pipes[1], how, 1);
 		_exit(lastval);
 	    }
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index 828a3d1..c3bedb0 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -641,6 +641,23 @@ F:Must be tested with a top-level script rather than source or function
 >TERM
 >EXIT
 
+  # Should not get "hello" in the single quotes.
+  (
+    trap "echo hello" EXIT;
+    { :; } | { read line; print "'$line'"; }
+  )
+0:EXIT trap not called in LHS of pipeline: Shell construct on LHS
+>''
+>hello
+
+  (
+    trap "echo hello" EXIT;
+    cat </dev/null | { read line; print "'$line'"; }
+  )
+0:EXIT trap not called in LHS of pipeline: External command on LHS
+>''
+>hello
+
 %clean
 
   rm -f TRAPEXIT