From 95b4d6a31f5255987db5f8005f557bb63aa3a2cb Mon Sep 17 00:00:00 2001 From: Paul Ackersviller Date: Sun, 18 Nov 2007 21:57:32 +0000 Subject: Merge of 23460/23461: fix longstanding problem with multios attached to a subshell process. --- Src/exec.c | 51 +++++++++++++++++++++++++++++++++++++++++++++------ Src/jobs.c | 9 +++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) (limited to 'Src') 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) -- cgit 1.4.1