Re: bug with named pipes and process substitution

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Wed, 22 Jul 2015 10:09:44 +0100
Peter Stephenson <p.stephenson@xxxxxxxxxxx> wrote:
> The call we need is probably pipecleanfilelist(...) from the second
> commit, if we can find where to put it...

Best I could come up with was special handling of process substtitutions
after a fork... that looks like it does the business without side
effects:  the question is more whether this is general enough.

The rationale for this is that the process substitutions are always
handled locally in execcmd(), so we can be sure removing them in the
parent after the fork isn't doing any damage.  Other entries in filelist
don't have that guarantee.

Slightly tweaked test to remove dependencies but as it's actually more
complicated than the original, processwise, I think it should be OK.

One day this may even get committed...

pws

diff --git a/Src/exec.c b/Src/exec.c
index 4eee82b..7612d43 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3047,6 +3047,7 @@ execcmd(Estate state, int input, int output, int how, int last1)
 	    addproc(pid, text, 0, &bgtime);
 	    if (oautocont >= 0)
 		opts[AUTOCONTINUE] = oautocont;
+	    pipecleanfilelist(jobtab[thisjob].filelist, 1);
 	    return;
 	}
 	/* pid == 0 */
@@ -3492,7 +3493,7 @@ execcmd(Estate state, int input, int output, int how, int last1)
 
 	    if (is_shfunc) {
 		/* It's a shell function */
-		pipecleanfilelist(filelist);
+		pipecleanfilelist(filelist, 0);
 		execshfunc((Shfunc) hn, args);
 	    } else {
 		/* It's a builtin */
@@ -3682,7 +3683,7 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		DPUTS(varspc,
 		      "BUG: assignment before complex command");
 		list_pipe = 0;
-		pipecleanfilelist(filelist);
+		pipecleanfilelist(filelist, 0);
 		/* 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 948f61b..a71df68 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -1179,7 +1179,7 @@ addfilelist(const char *name, int fd)
 
 /**/
 void
-pipecleanfilelist(LinkList filelist)
+pipecleanfilelist(LinkList filelist, int proc_subst_only)
 {
     LinkNode node;
 
@@ -1188,7 +1188,9 @@ pipecleanfilelist(LinkList filelist)
     node = firstnode(filelist);
     while (node) {
 	Jobfile jf = (Jobfile)getdata(node);
-	if (jf->is_fd) {
+	if (jf->is_fd &&
+	    (!proc_subst_only ||
+	     fdtable[jf->u.fd] == FDT_PROC_SUBST)) {
 	    LinkNode next = nextnode(node);
 	    zclose(jf->u.fd);
 	    (void)remnode(filelist, node);
@@ -1433,7 +1435,7 @@ zwaitjob(int job, int wait_cmd)
 	     * we can't deadlock on the fact that those still exist, so
 	     * that's not a problem.
 	     */
-	    pipecleanfilelist(jn->filelist);
+	    pipecleanfilelist(jn->filelist, 0);
 	}
 	while (!errflag && jn->stat &&
 	       !(jn->stat & STAT_DONE) &&
@@ -1623,7 +1625,7 @@ spawnjob(void)
 	deletejob(jobtab + thisjob, 0);
     else {
 	jobtab[thisjob].stat |= STAT_LOCKED;
-	pipecleanfilelist(jobtab[thisjob].filelist);
+	pipecleanfilelist(jobtab[thisjob].filelist, 0);
     }
     thisjob = -1;
 }
diff --git a/Src/zsh.h b/Src/zsh.h
index 69fef33..a99c900 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1752,9 +1752,10 @@ struct tieddata {
 				  * necessarily want to match multiple
 				  * elements
 				  */
-#define SCANPM_ISVAR_AT   ((-1)<<15)	/* "$foo[@]"-style substitution
-					 * Only sign bit is significant
-					 */
+/* "$foo[@]"-style substitution
+ * Only sign bit is significant
+ */
+#define SCANPM_ISVAR_AT   ((int)(((unsigned int)-1)<<15))
 
 /*
  * Flags for doing matches inside parameter substitutions, i.e.
diff --git a/Test/D03procsubst.ztst b/Test/D03procsubst.ztst
index 7b87589..1fc09b1 100644
--- a/Test/D03procsubst.ztst
+++ b/Test/D03procsubst.ztst
@@ -126,3 +126,18 @@
   eval 'foo here is some output)'
 0:full alias expanded when substitution starts in alias
 >here is some output
+
+  if ! (mkpipe test_pipe >/dev/null 2>&1); then
+    ZTST_skip="mkpipe not available"
+  else
+    echo 1 | tee >(cat > test_pipe) | (){
+      local pipein
+      read pipein <test_pipe
+      print $pipein
+      read pipein
+      print $pipein
+    }
+  fi
+0:proc subst fd in forked subshell closed in parent
+>1
+>1