From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15588 invoked by alias); 21 Feb 2015 02:28:53 -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: 34590 Received: (qmail 15644 invoked from network); 21 Feb 2015 02:28:41 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-CMAE-Score: 0 X-CMAE-Analysis: v=2.1 cv=A7bb5BSD c=1 sm=1 tr=0 a=FT8er97JFeGWzr5TCOCO5w==:117 a=kj9zAlcOel0A:10 a=q2GGsy2AAAAA:8 a=oR5dmqMzAAAA:8 a=-9mUelKeXuEA:10 a=0HtSIViG9nkA:10 a=VM41hHZmxLuUr-VYn_4A:9 a=paKJCDAtonhtCr5y:21 a=NmJ9mQd0BsKNyt4Z:21 a=CjuIK1q_8ugA:10 From: Bart Schaefer Message-id: <150220182812.ZM11805@torch.brasslantern.com> Date: Fri, 20 Feb 2015 18:28:12 -0800 In-reply-to: <150219092329.ZM17912@torch.brasslantern.com> Comments: In reply to Bart Schaefer "Re: deadlock in free() called from a signal handler" (Feb 19, 9:23am) References: <37490085.zXPQGCoLTl@kdudka.brq.redhat.com> <150219092329.ZM17912@torch.brasslantern.com> X-Mailer: OpenZMail Classic (0.9.2 24April2005) To: Kamil Dudka , zsh-workers@zsh.org Subject: PATCH Re: deadlock in free() called from a signal handler MIME-version: 1.0 Content-type: text/plain; charset=us-ascii On Feb 19, 9:23am, Bart Schaefer wrote: } } In every case I can remember so far, when this happens it means that we } ought to be using the signal queuing macros at a scope outside the call } to the malloc library. } [...] } So ... per my very first remark above, it's probably worth examining the } context in execcmd() to see if signal queuing is appropriate there, or } if we just need it in setunderscore(). But it may also make sense to } replace that one last malloc() and put queuing in zfree()/zsfree() too. (Above edited for silly typo.) There doesn't appear to be an appropriate scope for queuing signals in execcmd(). However, setunderscore() and all of the functions in text.c manipulate global state, so they should be considered non-reentrant. The following patch does not yet put signal queuing into zfree(), but does wrap it around setunderscore() and the three non-static functions in text.c, as well as replace that one remaining malloc() with zalloc(). Hopefully this doesn't slow down execution noticeably. diff --git a/Src/exec.c b/Src/exec.c index 947b815..1a6149a 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -2337,6 +2337,7 @@ addvars(Estate state, Wordcode pc, int addflags) void setunderscore(char *str) { + queue_signals(); if (str && *str) { int l = strlen(str) + 1, nl = (l + 31) & ~31; @@ -2354,6 +2355,7 @@ setunderscore(char *str) *zunderscore = '\0'; underscoreused = 1; } + unqueue_signals(); } /* These describe the type of expansions that need to be done on the words @@ -5319,7 +5321,7 @@ execsave(void) { struct execstack *es; - es = (struct execstack *) malloc(sizeof(struct execstack)); + es = (struct execstack *) zalloc(sizeof(struct execstack)); es->list_pipe_pid = list_pipe_pid; es->nowait = nowait; es->pline_level = pline_level; diff --git a/Src/text.c b/Src/text.c index 75f642c..b58c251 100644 --- a/Src/text.c +++ b/Src/text.c @@ -173,6 +173,8 @@ getpermtext(Eprog prog, Wordcode c, int start_indent) { struct estate s; + queue_signals(); + if (!c) c = prog->prog; @@ -193,6 +195,9 @@ getpermtext(Eprog prog, Wordcode c, int start_indent) *tptr = '\0'; freeeprog(prog); /* mark as unused */ untokenize(tbuf); + + unqueue_signals(); + return tbuf; } @@ -206,6 +211,8 @@ getjobtext(Eprog prog, Wordcode c) struct estate s; + queue_signals(); + if (!c) c = prog->prog; @@ -224,6 +231,9 @@ getjobtext(Eprog prog, Wordcode c) *tptr = '\0'; freeeprog(prog); /* mark as unused */ untokenize(jbuf); + + unqueue_signals(); + return jbuf; } @@ -883,6 +893,9 @@ getredirs(LinkList redirs) ">", ">|", ">>", ">>|", "&>", "&>|", "&>>", "&>>|", "<>", "<", "<<", "<<-", "<<<", "<&", ">&", NULL /* >&- */, "<", ">" }; + + queue_signals(); + taddchr(' '); for (n = firstnode(redirs); n; incnode(n)) { Redir f = (Redir) getdata(n); @@ -970,4 +983,6 @@ getredirs(LinkList redirs) } } tptr--; + + unqueue_signals(); } -- Barton E. Schaefer