zsh-workers
 help / color / mirror / code / Atom feed
From: Tycho Kirchner <tychokirchner@mail.de>
To: Bart Schaefer <schaefer@brasslantern.com>
Cc: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: exit_function - strange behavior
Date: Mon, 1 Nov 2021 10:47:10 +0100	[thread overview]
Message-ID: <ce0e92c3-f0c7-d231-cb5d-e415cec306f6@mail.de> (raw)
In-Reply-To: <CAH+w=7ZcGXnFBDwoK2hrgdg0uz7vTaKftCmYQsAhpm0eWdGytQ@mail.gmail.com>



Am 01.11.21 um 00:52 schrieb Bart Schaefer:
> On Mon, Oct 25, 2021 at 5:53 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>>
>> There's definitely a bug in this somewhere because if the last command
>> executed by the final subshell [is] an anonymous function call:
>>
>> zsh -fc '...; ( () { return 123 } )'
>>
>> then the zshexit hook is NEVER called.
> 
> When an anonymous function (or probably any function) calls "exit", we
> pass through this bit of code in builtin.c bin_break():
> 
> 5719             *
> 5720             * If we are already exiting... give this all up as
> 5721             * a bad job.
> 5722             */
> 5723            if (stopmsg || (zexit(0, ZEXIT_DEFERRED), !stopmsg)) {
> 5724            retflag = 1;
> 5725            breaks = loops;
> 5726            exit_pending = 1;
> 5727            exit_level = locallevel;
> 5728            exit_val = num;
> 
> With ZEXIT_DEFERRED, zexit() always bails out here:
> 
> 5842     /* Positive shell_exiting means we have been here before */
> 5843     if (from_where == ZEXIT_DEFERRED ||
> 5844         (shell_exiting++ && from_where != ZEXIT_NORMAL))
> 5845         return;
> 
> We then eventually call zexit(exit_val, ZEXIT_NORMAL) from doshfunc().
> 
> If instead the function calls "return 123" and is the last command in
> the subshell, we pass through this branch of execcmd_exec():
> 
> 4239     if (forked) {
> 4240         /*
> 4241          * So what's going on here then?  Well, I'm glad you asked.
> [...]
> 4263          */
> 4264         for (i = 0; i < 10; i++)
> 4265             if (fdtable[i] != FDT_UNUSED)
> 4266                 close(i);
> 4267         closem(FDT_UNUSED, 1);
> 4268         if (thisjob != -1)
> 4269             waitjobs();
> 4270         _realexit();
> 4271     }
> 
> The call to _realexit() bypasses the hook.  This is the expected
> behavior for "falling off the end" of a subshell, rather than
> explicitly "exit"-ing.
> 
> However, in
>    zsh -fc '( () { return 123 } )'
> the parent shell optimizes away the subshell and we arrive here:
> 
> 3596             /*
> 3597              * If we are in a subshell environment anyway, say
> we're forked,
> 3598              * even if we're actually not forked because we know the
> 3599              * subshell is exiting.  This ensures SHLVL reflects
> the current
> 3600              * shell, and also optimises out any save/restore we'd need to
> 3601              * do if we were returning to the main shell.
> 3602              */
> 3603             if (type == WC_SUBSH)
> 3604                 forked = 1;
> 
> This should not happen when there is an exit trap or an exit hook, I
> think?  However, I'm not following the comment reference to SHLVL --
> why would it not reflect the correct thing?  I suppose the actual
> correct thing is higher up the call stack, where we should not assert
> that the subshell is exiting if the parent shell still has traps or
> hooks to process, so that we really have forked here.
> 
> As to this:
> 
>> You can avoid that by having the subshell end with "return 123"
>> instead of "exit 123", except for some reason (possibly a bug) when
>> the -c option is used in which case return behaves like exit again.
> 
> bin_break() treats "return" as synonymous with "exit" here:
> 
> 5683     case BIN_RETURN:
> 5684         if ((isset(INTERACTIVE) && isset(SHINSTDIN))
> 5685             || locallevel || sourcelevel) {
> [...]
> 5699             return lastval;
> 5700         }
> 5701         zexit(num, ZEXIT_NORMAL);       /* else treat return as
> logout/exit */
> 
> To Tycho's original question, this means there is no way to have an
> exit hook or trap called exactly when the original shell exits.  You
> can't forcibly finish a subshell before it "falls off the end" without
> possibly invoking the trap and hook, and you can't even reliably test
> $ZSH_SUBSHELL inside the hook, because the parent might optimize out a
> fork without decrementing that.
> 
> [[ $sysparams[pid] = $$ ]] almost gets there, except that you can't
> assure the hook itself won't be skipped by a function that calls exit.
> 

Bart, thanks for diving into this, I hope it can be fixed. I just wanted 
to point out that the documentation of $ZSH_SUBSHELL needs to be fixed 
as well:

ZSH_SUBSHELL

     Readonly integer. Initially zero, incremented each time the shell 
forks to create a subshell for executing code. Hence ‘(print 
$ZSH_SUBSHELL)’ and ‘print $(print $ZSH_SUBSHELL)’ output 1, while ‘( 
(print $ZSH_SUBSHELL) )’ outputs 2.

So $ZSH_SUBSHELL may also be incremented, if the shell did _not_ fork:

% zsh -fc 'zmodload zsh/system; echo parent_pid: $$;  echo 
parent_subshell $ZSH_SUBSHELL ; ( echo  sysparams_pid: $sysparams[pid]; 
read -d " " pid < /proc/self/stat; echo proc_pid:  $pid; echo 
child_subshell: $ZSH_SUBSHELL ; exit 123 )'
parent_pid: 20562
parent_subshell 0
sysparams_pid: 20562
proc_pid: 20562
child_subshell: 1

Anyway, I think it is ok for $ZSH_SUBSHELL to be inconsistent with 
$sysparams[pid] as the fork-optimization is an implementation detail and 
most users probably want to know whether being in enclosed '(  )'.



      reply	other threads:[~2021-11-01  9:48 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-25 23:28 Tycho Kirchner
2021-10-26  0:53 ` Bart Schaefer
2021-10-31 23:52   ` Bart Schaefer
2021-11-01  9:47     ` Tycho Kirchner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ce0e92c3-f0c7-d231-cb5d-e415cec306f6@mail.de \
    --to=tychokirchner@mail.de \
    --cc=schaefer@brasslantern.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).