23460: fix problem with ( stuff ) >multio1 >multio2

4 files changed, 62 insertions, 5 deletions

diff --git a/ChangeLog b/ChangeLog
index 5d521e898..71f51a966 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2007-05-23  Peter Stephenson  <pws@csr.com>
+
+	* 23460: Src/exec.c, Src/jobs.c, Test/E01options.ztst:
+	fix longstanding problem with multios attached to a
+	subshell process.
+
 2007-05-22  Peter Stephenson  <pws@csr.com>
 
 	* Phil Pennock: 23450: Src/Zle/zleparameter.yo: undefined
diff --git a/Src/exec.c b/Src/exec.c
index a462a4ee9..5aa3dba12 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1098,10 +1098,10 @@ execpline(Estate state, wordcode slcode, int how, int last1)
     child_block();
 
     /*
-     * Get free entry in job table and initialize it.
-     * This is currently the only call to initjob(), so this
-     * is also the only place where we can expand the job table
-     * under us.
+     * Get free entry in job table and initialize it.  This is currently
+     * the only call to initjob() (apart from a minor exception in
+     * clearjobtab()), so this is also the only place where we can
+     * expand the job table under us.
      */
     if ((thisjob = newjob = initjob()) == -1) {
 	child_unblock();
@@ -2816,8 +2816,38 @@ execcmd(Estate state, int input, int output, int how, int last1)
     }
 
   err:
-    if (forked)
+    if (forked) {
+	/*
+	 * So what's going on here then?  Well, I'm glad you asked.
+	 *
+	 * If we create multios for use in a subshell we do
+	 * this after forking, in this function above.  That
+	 * means that the current (sub)process is responsible
+	 * for clearing them up.  However, the processes won't
+	 * go away until we have closed the fd's talking to them.
+	 * Since we're about to exit the shell there's nothing
+	 * to stop us closing all fd's (including the ones 0 to 9
+	 * that we usually leave alone).
+	 *
+	 * Then we wait for any processes.  When we forked,
+	 * we cleared the jobtable and started a new job just for
+	 * any oddments like this, so if there aren't any we won't
+	 * need to wait.  The result of not waiting is that
+	 * the multios haven't flushed the fd's properly, leading
+	 * to obscure missing data.
+	 *
+	 * It would probably be cleaner to ensure that the
+	 * parent shell handled multios, but that requires
+	 * some architectural changes which are likely to be
+	 * hairy.
+	 */
+	for (i = 0; i < 10; i++)
+	    if (fdtable[i] != FDT_UNUSED)
+		close(i);
+	closem(FDT_UNUSED);
+	waitjobs();
 	_exit(lastval);
+    }
     fixfds(save);
 
  done:
diff --git a/Src/jobs.c b/Src/jobs.c
index 53e654c2b..b02922a03 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -1296,6 +1296,15 @@ clearjobtab(int monitor)
 
     memset(jobtab, 0, jobtabsize * sizeof(struct job)); /* zero out table */
     maxjob = 0;
+
+    /*
+     * Although we don't have job control in subshells, we
+     * sometimes needs control structures for other purposes such
+     * as multios.  Grab a job for this purpose; any will do
+     * since we've freed them all up (so there's no question
+     * of problems with the job table size here).
+     */
+    thisjob = initjob();
 }
 
 static int initnewjob(int i)
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index 1fbe0cc93..0ce82b3ab 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -655,6 +655,18 @@
 >This is in1
 >This is in2
 
+# This is trickier than it looks.  There's a hack at the end of
+# execcmd() to catch the multio processes attached to the
+# subshell, which otherwise sort of get lost in the general turmoil.
+# Without that, the multios aren't synchronous with the subshell
+# or the main shell starting the "cat", so the output files appear
+# empty.
+  setopt multios
+  ( echo hello ) >multio_out1 >multio_out2 && cat multio_out*
+0:Multios attached to a subshell
+>hello
+>hello
+
 # tried this with other things, but not on its own, so much.
   unsetopt nomatch
   print with nonomatch: flooble*