[BUG] exec + builtin and traps

[BUG] exec + builtin and traps

[Vincent Lefevre]

For zsh 5.4.1, the zshbuiltins(1) man page says:

    exec [ -cl ] [ -a argv0 ] [ command [ arg ... ] ]
           Replace the current shell with command rather than forking.
           If command is a shell builtin command or a shell  function,
           the shell executes it, then immediately exits.

But consider the following script:

----------------------------------------
#!/usr/bin/env zsh

trap 'echo foo' USR1

echo $$
( sleep 1; kill -USR1 $$ ) &
exec eval sleep 2
----------------------------------------

"foo" is printed, meaning that the shell does not immediately exit
just after the command is executed, contrary to what the man page
says.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: [BUG] exec + builtin and traps

[Peter Stephenson]

On Tue, 12 Sep 2017 12:02:57 +0200
Vincent Lefevre <vincent@vinc17.net> wrote:
> For zsh 5.4.1, the zshbuiltins(1) man page says:
> 
>     exec [ -cl ] [ -a argv0 ] [ command [ arg ... ] ]
>            Replace the current shell with command rather than forking.
>            If command is a shell builtin command or a shell  function,
>            the shell executes it, then immediately exits.

It means immediately after the command has finished executing, not while
it's executing.

> #!/usr/bin/env zsh
> 
> trap 'echo foo' USR1
> 
> echo $$
> ( sleep 1; kill -USR1 $$ ) &
> exec eval sleep 2

So in your case it's still exectuing the eval sleep 2.

I don't actually understand your interpretation --- given eval is a
builtin and the eval is going to take 2 seconds because it dispatches to
a sleep, how could the shell exit before that?

pws

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index 3afe990..d6aa078 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -669,7 +669,7 @@ an empty string or whitespace) the return status is zero.
 item(tt(exec) [ tt(-cl) ] [ tt(-a) var(argv0) ] [ var(command) [ var(arg) ... ] ])(
 Replace the current shell with var(command) rather than forking.
 If var(command) is a shell builtin command or a shell function,
-the shell executes it, then immediately exits.
+the shell executes it, and exits when the command is complete.
 
 With tt(-c) clear the environment; with tt(-l) prepend tt(-) to the
 tt(argv[0]) string of the command executed (to simulate a login shell);

Re: [BUG] exec + builtin and traps

[Vincent Lefevre]

On 2017-09-12 11:19:06 +0100, Peter Stephenson wrote:
> On Tue, 12 Sep 2017 12:02:57 +0200
> Vincent Lefevre <vincent@vinc17.net> wrote:
> > For zsh 5.4.1, the zshbuiltins(1) man page says:
> > 
> >     exec [ -cl ] [ -a argv0 ] [ command [ arg ... ] ]
> >            Replace the current shell with command rather than forking.
> >            If command is a shell builtin command or a shell  function,
> >            the shell executes it, then immediately exits.
> 
> It means immediately after the command has finished executing, not while
> it's executing.
> 
> > #!/usr/bin/env zsh
> > 
> > trap 'echo foo' USR1
> > 
> > echo $$
> > ( sleep 1; kill -USR1 $$ ) &
> > exec eval sleep 2
> 
> So in your case it's still exectuing the eval sleep 2.

No, by default (with TRAPS_ASYNC unset), traps are run *after* the
child process has exited:

    TRAPS_ASYNC
        While  waiting  for a program to exit, handle signals and run
        traps immediately.  Otherwise the trap is run after  a  child
        process  has  exited.  Note this does not affect the point at
        which traps are run for any case other than when the shell is
        waiting for a child process.

But if the shell immediately exits after the command has been
executed, this means that traps would not be run.

> I don't actually understand your interpretation --- given eval is a
> builtin and the eval is going to take 2 seconds because it dispatches to
> a sleep, how could the shell exit before that?

See above.

> diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
> index 3afe990..d6aa078 100644
> --- a/Doc/Zsh/builtins.yo
> +++ b/Doc/Zsh/builtins.yo
> @@ -669,7 +669,7 @@ an empty string or whitespace) the return status is zero.
>  item(tt(exec) [ tt(-cl) ] [ tt(-a) var(argv0) ] [ var(command) [ var(arg) ... ] ])(
>  Replace the current shell with var(command) rather than forking.
>  If var(command) is a shell builtin command or a shell function,
> -the shell executes it, then immediately exits.
> +the shell executes it, and exits when the command is complete.

This still does not describe the observed behavior when TRAPS_ASYNC
is unset.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: [BUG] exec + builtin and traps

[Peter Stephenson]

On Tue, 12 Sep 2017 12:39:19 +0200
Vincent Lefevre <vincent@vinc17.net> wrote:
> On 2017-09-12 11:19:06 +0100, Peter Stephenson wrote:
> > On Tue, 12 Sep 2017 12:02:57 +0200
> > Vincent Lefevre <vincent@vinc17.net> wrote:
> > > For zsh 5.4.1, the zshbuiltins(1) man page says:
> > > 
> > >     exec [ -cl ] [ -a argv0 ] [ command [ arg ... ] ]
> > >            Replace the current shell with command rather than forking.
> > >            If command is a shell builtin command or a shell  function,
> > >            the shell executes it, then immediately exits.
> > 
> > It means immediately after the command has finished executing, not while
> > it's executing.
> > 
> > > #!/usr/bin/env zsh
> > > 
> > > trap 'echo foo' USR1
> > > 
> > > echo $$
> > > ( sleep 1; kill -USR1 $$ ) &
> > > exec eval sleep 2
> > 
> > So in your case it's still exectuing the eval sleep 2.
> 
> No, by default (with TRAPS_ASYNC unset), traps are run *after* the
> child process has exited:

True, but the *builtin* is an eval list, that only terminates when it
has executed an arbitrary set of other commands.  The trap is executed
at the end of this list, before control returns back to eval.  We are
not exec'ing sleep here, we are exec'ing eval, so there is time after
the *child* process is executed, as in the TRAPS_ASYNC doc you quote.

eval, unlike exec, is not an permanent handoff of control to the
following command.

pws

Re: [BUG] exec + builtin and traps

[Vincent Lefevre]

On 2017-09-12 11:50:06 +0100, Peter Stephenson wrote:
> On Tue, 12 Sep 2017 12:39:19 +0200
> Vincent Lefevre <vincent@vinc17.net> wrote:
> > No, by default (with TRAPS_ASYNC unset), traps are run *after* the
> > child process has exited:
> 
> True, but the *builtin* is an eval list, that only terminates when it
> has executed an arbitrary set of other commands.  The trap is executed
> at the end of this list, before control returns back to eval.  We are
> not exec'ing sleep here, we are exec'ing eval, so there is time after
> the *child* process is executed, as in the TRAPS_ASYNC doc you quote.
> 
> eval, unlike exec, is not an permanent handoff of control to the
> following command.

OK, this was a bit confusing from the description of "eval" by POSIX,
which describes it as a way to construct a command:

  The eval utility shall construct a command by concatenating
  arguments together, separating each with a <space> character.
  The constructed command shall be read and executed by the shell.

BTW, the way zsh handles "eval" is still strange:

----------------------------------------
trap 'echo foo' USR1
( sleep 1; kill -USR1 $$; ) &
eval "wait && echo bar"
echo OK
----------------------------------------

outputs

foo
bar
OK

----------------------------------------
trap 'echo foo' USR1
( sleep 1; kill -USR1 $$; ) &
wait && echo bar
echo OK
----------------------------------------

outputs

foo
OK

All the other shells output

foo
OK

in both cases.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: [BUG] exec + builtin and traps

[Peter Stephenson]

On Tue, 12 Sep 2017 13:42:20 +0200
Vincent Lefevre <vincent@vinc17.net> wrote:
> BTW, the way zsh handles "eval" is still strange:
> 
> ----------------------------------------
> trap 'echo foo' USR1
> ( sleep 1; kill -USR1 $$; ) &
> eval "wait && echo bar"
> echo OK
> ----------------------------------------
> 
> outputs
> 
> foo
> bar
> OK

That'll be another effect of NO_ASYNC_TRAPS --- the list being executed
is what's in quotes, and it's after that that the trap gets run.  I
don't think NO_ASYNC_TRAPS behaviour is standardised.

pws

Re: [BUG] exec + builtin and traps

[Vincent Lefevre]

On 2017-09-12 12:51:39 +0100, Peter Stephenson wrote:
> On Tue, 12 Sep 2017 13:42:20 +0200
> Vincent Lefevre <vincent@vinc17.net> wrote:
> > BTW, the way zsh handles "eval" is still strange:
> > 
> > ----------------------------------------
> > trap 'echo foo' USR1
> > ( sleep 1; kill -USR1 $$; ) &
> > eval "wait && echo bar"
> > echo OK
> > ----------------------------------------
> > 
> > outputs
> > 
> > foo
> > bar
> > OK
> 
> That'll be another effect of NO_ASYNC_TRAPS --- the list being executed
> is what's in quotes, and it's after that that the trap gets run.

Why is "foo" (from the trap) printed before "bar", then?

> I don't think NO_ASYNC_TRAPS behaviour is standardised.

That's another reason why it would be useful to clearly document
when traps are run precisely.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: [BUG] exec + builtin and traps

[Peter Stephenson]

On Tue, 12 Sep 2017 14:02:37 +0200
Vincent Lefevre <vincent@vinc17.net> wrote:
> Why is "foo" (from the trap) printed before "bar", then?

OK, so it's on the *sub* list after the command before the &&
(wait) has executed successfully, but before the foo is executed.

> > I don't think NO_ASYNC_TRAPS behaviour is standardised.
> 
> That's another reason why it would be useful to clearly document
> when traps are run precisely.

Yes, it's a bit obscure and not really planned --- they just happen
"sufficienly often".

pws

trap, eval and wait (was: [BUG] exec + builtin and traps)

[Vincent Lefevre]

On 2017-09-12 13:10:54 +0100, Peter Stephenson wrote:
> On Tue, 12 Sep 2017 14:02:37 +0200
> Vincent Lefevre <vincent@vinc17.net> wrote:
> > Why is "foo" (from the trap) printed before "bar", then?
> 
> OK, so it's on the *sub* list after the command before the &&
> (wait) has executed successfully, but before the foo is executed.

I still don't understand. Actually, the real issue is that "wait"
should not have terminated successfully. Perhaps, this should be
clearer with:

------------------------------------------------------------
trap 'echo foo' USR1
( sleep 1; kill -USR1 $$; sleep 1; echo child ) &
eval "wait && echo bar"
echo OK
sleep 2
------------------------------------------------------------

After 1 second, the following is printed:

foo
bar
OK

And after another second:

child

So, this means that the "wait" has ended due to the USR1 signal, not
by a process termination. But then, the fact that "wait" terminates
with the exit status 0 does not seem to be correct.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: trap, eval and wait (was: [BUG] exec + builtin and traps)

[Peter Stephenson]

On Tue, 12 Sep 2017 16:21:45 +0200
Vincent Lefevre <vincent@vinc17.net> wrote:
> trap 'echo foo' USR1
> ( sleep 1; kill -USR1 $$; sleep 1; echo child ) &
> eval "wait && echo bar"
> echo OK
> sleep 2
> ------------------------------------------------------------
> 
> After 1 second, the following is printed:
> 
> foo
> bar
> OK
> 
> And after another second:
> 
> child
> 
> So, this means that the "wait" has ended due to the USR1 signal, not
> by a process termination. But then, the fact that "wait" terminates
> with the exit status 0 does not seem to be correct.

There's a funny here (yes, I know, we're all amazed) --- wait is
actually waiting for two jobs. One of them has the flags
STAT_BUILTIN|STAT_CURSH|STAT_NOPRINT, and it turns out this is the
second one it waits for and it returns status 0. The return status from
wait only reflects the last job waited for --- I believe that much is
standard.

Quite possibly any of the above flags should mean wait ignores the job,
but I've taken STAT_NOPRINT as the most logical to test here as it
basically means "user is not interested in this".

pws

diff --git a/Src/jobs.c b/Src/jobs.c
index 66dfb5a..226e7cf 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -2217,7 +2217,8 @@ bin_fg(char *name, char **argv, Options ops, int func)
 	    return 0;
 	} else {   /* Must be BIN_WAIT, so wait for all jobs */
 	    for (job = 0; job <= maxjob; job++)
-		if (job != thisjob && jobtab[job].stat)
+		if (job != thisjob && jobtab[job].stat &&
+		    !(jobtab[job].stat & STAT_NOPRINT))
 		    retval = zwaitjob(job, 1);
 	    unqueue_signals();
 	    return retval;

Re: trap, eval and wait (was: [BUG] exec + builtin and traps)

[Peter Stephenson]

On Tue, 12 Sep 2017 16:04:09 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> Quite possibly any of the above flags should mean wait ignores the job,
> but I've taken STAT_NOPRINT as the most logical to test here as it
> basically means "user is not interested in this".

I've committed this but I'm wondering if it would be sensible to add
STAT_DONE here? There's nothing to wait for if the job is done.
(This case is only for "wait" without an argument, so just wait for
everything we think is outstanding.)

By the way, unrelated to the bug (though it works around it), if you're
really interested in the status of the last background job, I think you
need to use "wait $!" anyway --- otherwise if the job is already
finished, "wait" will simply return immediately with status 0.  Waiting
for $! is the only special case available to handle the race.

pws