From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1921 invoked by alias); 26 Apr 2017 19:52:26 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 41012 Received: (qmail 8711 invoked from network); 26 Apr 2017 19:52:26 -0000 X-Qmail-Scanner-Diagnostics: from know-smtprelay-omc-11.server.virginmedia.net by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(80.0.253.75):SA:0(-2.8/5.0):. Processed in 1.76844 secs); 26 Apr 2017 19:52:26 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-2.8 required=5.0 tests=RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_PASS,T_DKIM_INVALID autolearn=unavailable autolearn_force=no version=3.4.1 X-Envelope-From: p.w.stephenson@ntlworld.com X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: pass (ns1.primenet.com.au: SPF record at _smtprelay.virginmedia.com designates 80.0.253.75 as permitted sender) X-Originating-IP: [86.21.219.59] X-Authenticated-User: p.w.stephenson@ntlworld.com X-Spam: 0 X-Authority: v=2.1 cv=cupm6AMi c=1 sm=1 tr=0 a=utowdAHh8RITBM/6U1BPxA==:117 a=utowdAHh8RITBM/6U1BPxA==:17 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=kj9zAlcOel0A:10 a=NLZqzBF-AAAA:8 a=1KTMFzqVivNkvSjxXv4A:9 a=CjuIK1q_8ugA:10 a=wW_WBVUImv98JQXhvVPZ:22 Date: Wed, 26 Apr 2017 20:52:15 +0100 From: Peter Stephenson To: zsh-workers@zsh.org Subject: Re: Call stack issues when running trap handler Message-ID: <20170426205215.73e722ce@ntlworld.com> In-Reply-To: <20170426202729.12cf4bb8@ntlworld.com> References: <87efwlijrl.fsf@wirrsal.net> <20170426202729.12cf4bb8@ntlworld.com> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.28; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ntlworld.com; s=meg.feb2017; t=1493236336; bh=D+EdxQnGeYOFqLdKl7n0jvypmXScVzqQsyjjy94HGv4=; h=Date:From:To:Subject:In-Reply-To:References; b=dXy1kP1/zlOkfMgQqVPwGqC+2mHYSb3HduPHj+UWxjn1BtgWck8Fus9PQg7E2rHUO oSkWu2vhxqmiyO1sKfVt2Qmw9oRopJvBTEEFLbcPpfFiDL/WAxG4ryrCZcqvdxYQ4a YyRBX4bP1j4xuBj94ZKmJxTwJHb+pngvyikKI5yoqvBk9AmLDYJ55anZcCStreIRBj dlFN8zOkN4N6O3UPqyz7j1Bl5PGQ5+40UmWMQWo655MOfgC+wbJAVgXWvIAJXHPgu0 ImJBb2ERB/GVV+eLVHKUo77w54Y6inQpeNjy0Llqs7qntQidcHd8XCBio++qJPHKby I/xfE6qvsaTUg== On Wed, 26 Apr 2017 20:27:29 +0100 Peter Stephenson wrote: > This I can fix. I think I like this better. shell_exiting looks like a good way of ensuring we don't tie ourselves in knots with exits, independent of in_exit_trap which we still need for the original fix. The interaction is that when we commit to exiting shell_exiting goes to -1; then we won't set exit_pending in zexit(); that means we don't hit the case that caused us to exit early. Or something. pws diff --git a/Src/builtin.c b/Src/builtin.c index b2e552d..063644e 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -5500,7 +5500,7 @@ bin_break(char *name, char **argv, UNUSED(Options ops), int func) } /*FALLTHROUGH*/ case BIN_EXIT: - if (locallevel > forklevel) { + if (locallevel > forklevel && shell_exiting != -1) { /* * We don't exit directly from functions to allow tidying * up, in particular EXIT traps. We still need to perform @@ -5509,6 +5509,9 @@ bin_break(char *name, char **argv, UNUSED(Options ops), int func) * * If we are forked, we exit the shell at the function depth * at which we became a subshell, hence the comparison. + * + * If we are already exiting... give this all up as + * a bad job. */ if (stopmsg || (zexit(0,2), !stopmsg)) { retflag = 1; @@ -5555,6 +5558,14 @@ checkjobs(void) } } +/* + * -1 if the shell is already committed to exit. + * positive if zexit() was already called. + */ + +/**/ +int shell_exiting; + /* exit the shell. val is the return value of the shell. * * from_where is * 1 if zexit is called because of a signal @@ -5566,10 +5577,8 @@ checkjobs(void) mod_export void zexit(int val, int from_where) { - static int in_exit; - /* Don't do anything recursively: see below */ - if (in_exit == -1) + if (shell_exiting == -1) return; if (isset(MONITOR) && !stopmsg && from_where != 1) { @@ -5582,14 +5591,14 @@ zexit(int val, int from_where) } } /* Positive in_exit means we have been here before */ - if (from_where == 2 || (in_exit++ && from_where)) + if (from_where == 2 || (shell_exiting++ && from_where)) return; /* - * We're now committed to exiting. Set in_exit to -1 to + * We're now committed to exiting. Set shell_exiting to -1 to * indicate we shouldn't do any recursive processing. */ - in_exit = -1; + shell_exiting = -1; /* * We want to do all remaining processing regardless of preceding * errors, even user interrupts. diff --git a/Src/exec.c b/Src/exec.c index 978a32d..e0fc544 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -5688,8 +5688,11 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) * the only likely case where we need that second test is * when we have an "always" block. The endparamscope() has * already happened, hence the "+1" here. + * + * If we are in an exit trap, finish it first... we wouldn't set + * exit_pending if we were already in one. */ - if (exit_pending && exit_level >= locallevel+1) { + if (exit_pending && exit_level >= locallevel+1 && !in_exit_trap) { if (locallevel > forklevel) { /* Still functions to return: force them to do so. */ retflag = 1; diff --git a/Src/signals.c b/Src/signals.c index 68a7ae3..cad40f4 100644 --- a/Src/signals.c +++ b/Src/signals.c @@ -55,6 +55,11 @@ mod_export Eprog siglists[VSIGCOUNT]; /**/ mod_export int nsigtrapped; +/* Running an exit trap? */ + +/**/ +int in_exit_trap; + /* * Flag that exit trap has been set in POSIX mode. * The setter's expectation is therefore that it is run @@ -1435,7 +1440,13 @@ dotrap(int sig) dont_queue_signals(); + if (sig == SIGEXIT) + ++in_exit_trap; + dotrapargs(sig, sigtrapped+sig, funcprog); + if (sig == SIGEXIT) + --in_exit_trap; + restore_queue_signals(q); } diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst index 7bc0b48..7594012 100644 --- a/Test/C03traps.ztst +++ b/Test/C03traps.ztst @@ -756,6 +756,27 @@ F:Must be tested with a top-level script rather than source or function >'' >hello + $ZTST_testdir/../Src/zsh -f =(<<<" + trap handler EXIT + handler() { + echoa + echo b + } + echoa() { + echo a + } + exit0() { + exit + } + main() { + exit0 + } + main + ") +0:No early exit from nested function in EXIT trap. +>a +>b + %clean rm -f TRAPEXIT