about summary refs log tree commit diff
path: root/Src
diff options
context:
space:
mode:
authorPaul Ackersviller <packersv@users.sourceforge.net>2007-11-18 21:57:32 +0000
committerPaul Ackersviller <packersv@users.sourceforge.net>2007-11-18 21:57:32 +0000
commit95b4d6a31f5255987db5f8005f557bb63aa3a2cb (patch)
treef3c2a3f0ec252949cb776c8795dbb0de0b60a01d /Src
parent083359cd89d28415a909402e7f69708bb3a32aaa (diff)
downloadzsh-95b4d6a31f5255987db5f8005f557bb63aa3a2cb.tar.gz
zsh-95b4d6a31f5255987db5f8005f557bb63aa3a2cb.tar.xz
zsh-95b4d6a31f5255987db5f8005f557bb63aa3a2cb.zip
Merge of 23460/23461: fix longstanding problem with multios attached to a subshell process.
Diffstat (limited to 'Src')
-rw-r--r--Src/exec.c51
-rw-r--r--Src/jobs.c9
2 files changed, 54 insertions, 6 deletions
diff --git a/Src/exec.c b/Src/exec.c
index 61d21be90..1513368bb 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1047,10 +1047,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();
@@ -1462,20 +1462,29 @@ closemn(struct multio **mfds, int fd)
 	pid_t pid;
 	struct timeval bgtime;
 
+	/*
+	 * We need to block SIGCHLD in case the process
+	 * we are spawning terminates before the job table
+	 * is set up to handle it.
+	 */
+	child_block();
 	if ((pid = zfork(&bgtime))) {
 	    for (i = 0; i < mn->ct; i++)
 		zclose(mn->fds[i]);
 	    zclose(mn->pipe);
-	    if (pid == -1) { 
+	    if (pid == -1) {
 		mfds[fd] = NULL;
+		child_unblock();
 		return;
 	    }
 	    mn->ct = 1;
 	    mn->fds[0] = fd;
 	    addproc(pid, NULL, 1, &bgtime);
+	    child_unblock();
 	    return;
 	}
 	/* pid == 0 */
+	child_unblock();
 	closeallelse(mn);
 	if (mn->rflag) {
 	    /* tee process */
@@ -2533,8 +2542,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 2da44a4e8..274980321 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -1255,6 +1255,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)