From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7705 invoked by alias); 27 Oct 2013 22:19:06 -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: 31918 Received: (qmail 10533 invoked from network); 27 Oct 2013 22:19:00 -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 From: Bart Schaefer Message-id: <131027151858.ZM29064@torch.brasslantern.com> Date: Sun, 27 Oct 2013 15:18:58 -0700 In-reply-to: <20131027203347.15be6bc2@pws-pc.ntlworld.com> Comments: In reply to Peter Stephenson "Re: Multio deadlock (Re: multios doesn't work with 2>&1)" (Oct 27, 8:33pm) References: <20131027145917.GA5509@localhost.localdomain> <131027100137.ZM4100@torch.brasslantern.com> <20131027174645.6934d78d@pws-pc.ntlworld.com> <131027112724.ZM16426@torch.brasslantern.com> <131027123917.ZM27930@torch.brasslantern.com> <20131027203347.15be6bc2@pws-pc.ntlworld.com> X-Mailer: OpenZMail Classic (0.9.2 24April2005) To: "Zsh Hackers' List" Subject: Re: Multio deadlock (Re: multios doesn't work with 2>&1) MIME-version: 1.0 Content-type: text/plain; charset=us-ascii On Oct 27, 8:33pm, Peter Stephenson wrote: } Subject: Re: Multio deadlock (Re: multios doesn't work with 2>&1) } } > for (i = 0; i < 10; i++) } > - if (fdtable[i] != FDT_UNUSED) } > + if (i < 3 || fdtable[i] != FDT_UNUSED) } > close(i); } } Hmm... why don't we initialise fdtable[0..2] to FDT_INTERNAL at boot? I don't know. Is FDT_INTERNAL the correct flag? It supposedly means that those descriptors are "not visible to other processes" which does not sound right to me. I think they should be FDT_EXTERNAL by default? } (FDT_UNUSED = 0, FDT_INTERNAL = 1). However, I've alsoe looked later } and seen all three set to FDT_INTERNAL; not sure where that happens but } presumably something to do with redir. It happens in utils.c:redup() but I'm not sure it's strictly correct. FDT_INTERNAL means they'll be "closed on exec" in execcmd() which is not right in the general case, but maybe it is when they've been dup'd. } Shouldn't we just initialise them properly? That would be better, yes, but they get toggled between FDT_UNUSED and FDT_INTERNAL every time there's a redirection, first when the original fd is closed [zclose()] and then when restored again afterward [redup()]. So the question is, in the deadlock case why is fd 2 open but still marked FDT_UNUSED? Who dup'd onto it without going through redup()? Or is that not what happened? (Its really difficult to trace this with gdb because all of this happens in one of the child processes.) See patch below, done essentially on a guess. Interesting aside: If fdtable[x] starts out FDT_EXTERNAL and is then saved with movefd() and restored with redup(), its state changes to FDT_INTERNAL because movefd() does not preserve the original state [it marks everything internal], but redup() copies the state. } We could in principle detect if they're open by attempting to dup() and } seeing what fd we get. Not sure if that's worth it, but it might be } cleaner. You mean, do that in order to determine whether they are already "unused" when the shell starts up? I don't think that's necessary. If you mean do it to avoid close(), it won't help, errno will still get frobbed. The following does appear to prevent the deadlock, but probably is not yet sufficient because it doesn't check that fdtable[] is big enough. diff --git a/Src/exec.c b/Src/exec.c index 99c7eaa..2e57443 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -3124,6 +3124,7 @@ execcmd(Estate state, int input, int output, int how, int last1) if(fd == -2) fd = (fn->type == REDIR_MERGEOUT) ? coprocout : coprocin; fil = dup(fd); + fdtable[fil] = FDT_INTERNAL; } if (fil == -1) { char fdstr[4]; @@ -3150,9 +3151,10 @@ execcmd(Estate state, int input, int output, int how, int last1) O_WRONLY | O_APPEND | O_CREAT | O_NOCTTY, 0666); else fil = clobber_open(fn); - if(fil != -1 && IS_ERROR_REDIR(fn->type)) + if(fil != -1 && IS_ERROR_REDIR(fn->type)) { dfil = dup(fil); - else + fdtable[dfil] = FDT_INTERNAL; + } else dfil = 0; if (fil == -1 || dfil == -1) { if(fil != -1)