From aede5c52bf557f89d1d09fa5430186af6e295241 Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Sat, 21 Dec 2013 17:41:21 -0800 Subject: 32176: plug additional deadlock-inducing pipe descriptor leaks --- ChangeLog | 5 +++++ Src/exec.c | 5 ++++- Src/jobs.c | 38 +++++++++++++++++++++++++------------- Test/A05execution.ztst | 2 ++ 4 files changed, 36 insertions(+), 14 deletions(-) diff --git a/ChangeLog b/ChangeLog index c4760218e..9efa11950 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2013-12-21 Barton E. Schaefer + + * PWS + Bart: 32176: plug additional descriptor leaks causing + deadlock via different code paths; expand regression test + 2013-12-21 Peter Stephenson * Config/version.mk: update version to 5.0.4-dev-0 so as diff --git a/Src/exec.c b/Src/exec.c index 44800339f..f16cfd391 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -2389,7 +2389,7 @@ static void execcmd(Estate state, int input, int output, int how, int last1) { HashNode hn = NULL; - LinkList args; + LinkList args, filelist = NULL; LinkNode node; Redir fn; struct multio *mfds[10]; @@ -2911,6 +2911,7 @@ execcmd(Estate state, int input, int output, int how, int last1) flags |= ESUB_KEEPTRAP; if (type == WC_SUBSH && !(how & Z_ASYNC)) flags |= ESUB_JOB_CONTROL; + filelist = jobtab[thisjob].filelist; entersubsh(flags); close(synch[1]); forked = 1; @@ -3264,6 +3265,7 @@ execcmd(Estate state, int input, int output, int how, int last1) if (is_shfunc) { /* It's a shell function */ + pipecleanfilelist(filelist); execshfunc((Shfunc) hn, args); } else { /* It's a builtin */ @@ -3342,6 +3344,7 @@ execcmd(Estate state, int input, int output, int how, int last1) DPUTS(varspc, "BUG: assignment before complex command"); list_pipe = 0; + pipecleanfilelist(filelist); /* 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 371b8eb32..a32117217 100644 --- a/Src/jobs.c +++ b/Src/jobs.c @@ -1173,6 +1173,30 @@ addfilelist(const char *name, int fd) zaddlinknode(ll, jf); } +/* Clean up pipes no longer needed associated with a job */ + +/**/ +void +pipecleanfilelist(LinkList filelist) +{ + LinkNode node; + + if (!filelist) + return; + node = firstnode(filelist); + while (node) { + Jobfile jf = (Jobfile)getdata(node); + if (jf->is_fd) { + LinkNode next = nextnode(node); + zclose(jf->u.fd); + (void)remnode(filelist, node); + zfree(jf, sizeof(*jf)); + node = next; + } else + incnode(node); + } +} + /* Finished with list of files for a job */ /**/ @@ -1415,19 +1439,7 @@ zwaitjob(int job, int wait_cmd) * we can't deadlock on the fact that those still exist, so * that's not a problem. */ - LinkNode node = firstnode(jn->filelist); - while (node) { - Jobfile jf = (Jobfile)getdata(node); - if (jf->is_fd) { - LinkNode next = nextnode(node); - (void)remnode(jn->filelist, node); - zclose(jf->u.fd); - zfree(jf, sizeof(*jf)); - node = next; - } else { - incnode(node); - } - } + pipecleanfilelist(jn->filelist); } while (!errflag && jn->stat && !(jn->stat & STAT_DONE) && diff --git a/Test/A05execution.ztst b/Test/A05execution.ztst index 61d24fe1e..6abfd8b1f 100644 --- a/Test/A05execution.ztst +++ b/Test/A05execution.ztst @@ -207,10 +207,12 @@ F:This similar test was triggering a reproducible failure with pipestatus. coproc { read -Et 5 || kill -INT $$ } print -u $ZTST_fd 'This test takes 5 seconds to fail...' { printf "%d\n" {1..20000} } | ( read -E ) + hang(){ printf "%d\n" {2..20000} | cat }; hang | ( read -E ) print -p done read -Ep 0:Bug regression: piping a shell construct to an external process may hang >1 +>2 >done F:This test checks for a file descriptor leak that could cause the left F:side of a pipe to block on write after the right side has exited -- cgit 1.4.1