From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1202 invoked by alias); 21 Dec 2013 22:34:38 -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: 32175 Received: (qmail 9520 invoked from network); 21 Dec 2013 22:34:32 -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=-2.6 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:subject:message-id:in-reply-to :references:mime-version:content-type:content-transfer-encoding; bh=qtXQeYkOryY9UALzOBw9J+Sv+OhCAFZCVXAefPx+iko=; b=QWzVar6NVuKZUeVsfehKZ3GNY5+7bXr3krlSd8Nbzuv5pIQwILW5EPfvzaOI6rIIXF +X5aosScWYov/0Vta+pJ/mCtkMjnIXYCfyQWAidCb3RErsBlOWoNvA1kDwy/QXKbMQo6 8fgmtcBYt+EAcrBKx+0f7IaRbPCUCRp8waAPmqrYDw+0REIHN2WQNBeQhs4v/Y3BJ/Pb 6B9DKcO93EGkqEGAI9B31YXNRBo040/j6HDJkzThWGLYrn6b08b36Ok+yk+xBmD2p7T0 swATcu8vWp2krdBeUbihIwF8y9tkVL0DJinE3KAbTN8W+6tSptgdSGult6Ex0L8EE7xy W1Hw== X-Gm-Message-State: ALoCoQk+YmTMX4iS1IZYQFIBHpCZQ5CcjDHiI6UftQgP28r+UejjBPaw1cb/ze/p5IN7U0ObWc+9 X-Received: by 10.194.20.130 with SMTP id n2mr66282wje.62.1387665269111; Sat, 21 Dec 2013 14:34:29 -0800 (PST) X-ProxyUser-IP: 86.6.157.246 Date: Sat, 21 Dec 2013 22:34:24 +0000 From: Peter Stephenson To: zsh-workers@zsh.org Subject: Re: Fwd (potential regression in 5.0.3): Bug#732726: zsh function freeze Message-ID: <20131221223424.6b7a55a7@pws-pc.ntlworld.com> In-Reply-To: <131221125718.ZM24141@torch.brasslantern.com> References: <20131220192435.GE27889@sym.noone.org> <131220122701.ZM15525@torch.brasslantern.com> <20131220235149.GA21721@xvii.vinc17.org> <20131220235955.GB21721@xvii.vinc17.org> <20131221001235.GC21721@xvii.vinc17.org> <131220181950.ZM15385@torch.brasslantern.com> <131220194223.ZM29152@torch.brasslantern.com> <20131221180846.78a5a013@pws-pc.ntlworld.com> <131221125718.ZM24141@torch.brasslantern.com> X-Mailer: Claws Mail 3.8.0 (GTK+ 2.24.7; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Sat, 21 Dec 2013 12:57:18 -0800 Bart Schaefer wrote: > As far as I can tell this fixes the problem. It'd be nice to hear from > Vincent for confirmation but as that may not happen in time I'd say you > should go ahead. Unfortunately it (the patch in 32171) doesn't fix the problem for me. With foo() { ls -R / } foo | head I'm still getting a hang. I think we're forking inside execcmd() after adding pipes[0] to the filelist for thisjob. This subshell is what's going to form the LHS of the pipeline --- and we entersubsh() which will clear the job table. So I think we need to salvage the filelist from the job table and remove the pipe file descriptors in the danger cases, which I take to be the places where we were handling subsh_close in the old version of the code (where we are handling nested shell constructs of some sort). The following does seem to fix the hang here and not cause any new test failures. Note it includes your code change, but not your regression test (you hadn't pushed either yet last I looked). The change to zwaitjob() is just pulling out the common code shared with the two new cases. I'm not going to commit this and make a new release just on the basis of my guesses, however. diff --git a/Src/exec.c b/Src/exec.c index dccdc2b..f16cfd3 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -1691,6 +1691,7 @@ execpline2(Estate state, wordcode pcode, execcmd(state, input, output, how, last1 ? 1 : 2); else { int old_list_pipe = list_pipe; + int subsh_close = -1; Wordcode next = state->pc + (*state->pc), pc; wordcode code; @@ -1738,6 +1739,7 @@ execpline2(Estate state, wordcode pcode, } else { /* otherwise just do the pipeline normally. */ addfilelist(NULL, pipes[0]); + subsh_close = pipes[0]; execcmd(state, input, pipes[1], how, 0); } zclose(pipes[1]); @@ -1750,6 +1752,8 @@ execpline2(Estate state, wordcode pcode, execpline2(state, *state->pc++, how, pipes[0], output, last1); list_pipe = old_list_pipe; cmdpop(); + if (subsh_close != pipes[0]) + zclose(pipes[0]); } } @@ -2385,7 +2389,7 @@ static void execcmd(Estate state, int input, int output, int how, int last1) { HashNode hn = NULL; - LinkList args; + LinkList args, filelist = NULL; LinkNode node; Redir fn; struct multio *mfds[10]; @@ -2907,6 +2911,7 @@ execcmd(Estate state, int input, int output, int how, int last1) flags |= ESUB_KEEPTRAP; if (type == WC_SUBSH && !(how & Z_ASYNC)) flags |= ESUB_JOB_CONTROL; + filelist = jobtab[thisjob].filelist; entersubsh(flags); close(synch[1]); forked = 1; @@ -3260,6 +3265,7 @@ execcmd(Estate state, int input, int output, int how, int last1) if (is_shfunc) { /* It's a shell function */ + pipecleanfilelist(filelist); execshfunc((Shfunc) hn, args); } else { /* It's a builtin */ @@ -3338,6 +3344,7 @@ execcmd(Estate state, int input, int output, int how, int last1) DPUTS(varspc, "BUG: assignment before complex command"); list_pipe = 0; + pipecleanfilelist(filelist); /* If we're forked (and we should be), no need to return */ DPUTS(last1 != 1 && !forked, "BUG: not exiting?"); DPUTS(type != WC_SUBSH, "Not sure what we're doing."); diff --git a/Src/jobs.c b/Src/jobs.c index 371b8eb..a321172 100644 --- a/Src/jobs.c +++ b/Src/jobs.c @@ -1173,6 +1173,30 @@ addfilelist(const char *name, int fd) zaddlinknode(ll, jf); } +/* Clean up pipes no longer needed associated with a job */ + +/**/ +void +pipecleanfilelist(LinkList filelist) +{ + LinkNode node; + + if (!filelist) + return; + node = firstnode(filelist); + while (node) { + Jobfile jf = (Jobfile)getdata(node); + if (jf->is_fd) { + LinkNode next = nextnode(node); + zclose(jf->u.fd); + (void)remnode(filelist, node); + zfree(jf, sizeof(*jf)); + node = next; + } else + incnode(node); + } +} + /* Finished with list of files for a job */ /**/ @@ -1415,19 +1439,7 @@ zwaitjob(int job, int wait_cmd) * we can't deadlock on the fact that those still exist, so * that's not a problem. */ - LinkNode node = firstnode(jn->filelist); - while (node) { - Jobfile jf = (Jobfile)getdata(node); - if (jf->is_fd) { - LinkNode next = nextnode(node); - (void)remnode(jn->filelist, node); - zclose(jf->u.fd); - zfree(jf, sizeof(*jf)); - node = next; - } else { - incnode(node); - } - } + pipecleanfilelist(jn->filelist); } while (!errflag && jn->stat && !(jn->stat & STAT_DONE) && -- Peter Stephenson Web page now at http://homepage.ntlworld.com/p.w.stephenson/