From 9958684574bf8b0ecec6983cca57f3fa3dd7cd63 Mon Sep 17 00:00:00 2001 From: "Barton E. Schaefer" Date: Sun, 9 Aug 2015 00:50:36 -0700 Subject: 36022 fix bug that some loop constructs could not be interrupted, revise signal queueing There are two underlying ideas here: (1) Keeping signals queued around anything that's doing memory management (including push/pop of the heap) has become crucial. (2) Anytime the shell is going to run a command, be it buitin or external, it must be both safe and necessary to process any queued signals, so that the apparent order of signal arrival and command execution is preserved. --- Src/exec.c | 49 ++++++++++++++++++++++++++++++++++++++++++------- Src/init.c | 5 +++++ Src/input.c | 12 ++++++++++-- Src/loop.c | 41 ++++++++++++++++++++++++++++++++++++++--- Src/parse.c | 8 ++++++++ Src/signals.c | 8 ++++++-- 6 files changed, 109 insertions(+), 14 deletions(-) (limited to 'Src') diff --git a/Src/exec.c b/Src/exec.c index 7612d4303..a635c18ed 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -1121,10 +1121,14 @@ execsimple(Estate state) fflush(xtrerr); } lv = (errflag ? errflag : cmdoutval); - } else if (code == WC_FUNCDEF) { - lv = execfuncdef(state, NULL); } else { - lv = (execfuncs[code - WC_CURSH])(state, 0); + int q = queue_signal_level(); + dont_queue_signals(); + if (code == WC_FUNCDEF) + lv = execfuncdef(state, NULL); + else + lv = (execfuncs[code - WC_CURSH])(state, 0); + restore_queue_signals(q); } thisjob = otj; @@ -1158,6 +1162,8 @@ execlist(Estate state, int dont_change_job, int exiting) */ int oldnoerrexit = noerrexit; + queue_signals(); + cj = thisjob; old_pline_level = pline_level; old_list_pipe = list_pipe; @@ -1428,6 +1434,8 @@ sublist_done: /* Make sure this doesn't get executed again. */ sigtrapped[SIGEXIT] = 0; } + + unqueue_signals(); } /* Execute a pipeline. * @@ -1456,6 +1464,14 @@ execpline(Estate state, wordcode slcode, int how, int last1) else if (slflags & WC_SUBLIST_NOT) last1 = 0; + /* If trap handlers are allowed to run here, they may start another + * external job in the middle of us starting this one, which can + * result in jobs being reaped before their job table entries have + * been initialized, which in turn leads to waiting forever for + * jobs that no longer exist. So don't do that. + */ + queue_signals(); + pj = thisjob; ipipe[0] = ipipe[1] = opipe[0] = opipe[1] = 0; child_block(); @@ -1468,6 +1484,7 @@ execpline(Estate state, wordcode slcode, int how, int last1) */ if ((thisjob = newjob = initjob()) == -1) { child_unblock(); + unqueue_signals(); return 1; } if (how & Z_TIMED) @@ -1523,6 +1540,7 @@ execpline(Estate state, wordcode slcode, int how, int last1) else spawnjob(); child_unblock(); + unqueue_signals(); /* Executing background code resets shell status */ return lastval = 0; } else { @@ -1580,15 +1598,18 @@ execpline(Estate state, wordcode slcode, int how, int last1) } if (!(jn->stat & STAT_LOCKED)) { updated = hasprocs(thisjob); - waitjobs(); + waitjobs(); /* deals with signal queue */ child_block(); } else updated = 0; if (!updated && list_pipe_job && hasprocs(list_pipe_job) && !(jobtab[list_pipe_job].stat & STAT_STOPPED)) { + int q = queue_signal_level(); child_unblock(); + dont_queue_signals(); child_block(); + restore_queue_signals(q); } if (list_pipe_child && jn->stat & STAT_DONE && @@ -1672,6 +1693,7 @@ execpline(Estate state, wordcode slcode, int how, int last1) break; } child_unblock(); + unqueue_signals(); if (list_pipe && (lastval & 0200) && pj >= 0 && (!(jn->stat & STAT_INUSE) || (jn->stat & STAT_DONE))) { @@ -3391,6 +3413,7 @@ execcmd(Estate state, int input, int output, int how, int last1) fflush(xtrerr); } } else if (isset(EXECOPT) && !errflag) { + int q = queue_signal_level(); /* * We delay the entersubsh() to here when we are exec'ing * the current shell (including a fake exec to run a builtin then @@ -3431,11 +3454,14 @@ execcmd(Estate state, int input, int output, int how, int last1) } else redir_prog = NULL; + dont_queue_signals(); lastval = execfuncdef(state, redir_prog); + restore_queue_signals(q); } else if (type >= WC_CURSH) { if (last1 == 1) do_exec = 1; + dont_queue_signals(); if (type == WC_AUTOFN) { /* * We pre-loaded this to get any redirs. @@ -3444,6 +3470,7 @@ execcmd(Estate state, int input, int output, int how, int last1) lastval = execautofn_basic(state, do_exec); } else lastval = (execfuncs[type - WC_CURSH])(state, do_exec); + restore_queue_signals(q); } else if (is_builtin || is_shfunc) { LinkList restorelist = 0, removelist = 0; /* builtin or shell function */ @@ -3610,7 +3637,9 @@ execcmd(Estate state, int input, int output, int how, int last1) } state->pc = opc; } + dont_queue_signals(); lastval = execbuiltin(args, assigns, (Builtin) hn); + restore_queue_signals(q); fflush(stdout); if (save[1] == -2) { if (ferror(stdout)) { @@ -4820,11 +4849,9 @@ execshfunc(Shfunc shf, LinkList args) if ((osfc = sfcontext) == SFC_NONE) sfcontext = SFC_DIRECT; xtrerr = stderr; - unqueue_signals(); doshfunc(shf, args, 0); - queue_signals(); sfcontext = osfc; free(cmdstack); cmdstack = ocs; @@ -5039,6 +5066,8 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) static int funcdepth; #endif + queue_signals(); /* Lots of memory and global state changes coming */ + pushheap(); oargv0 = NULL; @@ -5261,6 +5290,8 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval) } popheap(); + unqueue_signals(); + /* * Exit with a tidy up. * Only leave if we're at the end of the appropriate function --- @@ -5299,6 +5330,8 @@ runshfunc(Eprog prog, FuncWrap wrap, char *name) int cont, ouu; char *ou; + queue_signals(); + ou = zalloc(ouu = underscoreused); if (ou) memcpy(ou, zunderscore, underscoreused); @@ -5320,12 +5353,14 @@ runshfunc(Eprog prog, FuncWrap wrap, char *name) wrap = wrap->next; } startparamscope(); - execode(prog, 1, 0, "shfunc"); + execode(prog, 1, 0, "shfunc"); /* handles signal unqueueing */ if (ou) { setunderscore(ou); zfree(ou, ouu); } endparamscope(); + + unqueue_signals(); } /* Search fpath for an undefined function. Finds the file, and returns the * diff --git a/Src/init.c b/Src/init.c index 2ef90992d..f2021f073 100644 --- a/Src/init.c +++ b/Src/init.c @@ -105,6 +105,7 @@ loop(int toplevel, int justonce) Eprog prog; int err, non_empty = 0; + queue_signals(); pushheap(); if (!toplevel) zcontext_save(); @@ -126,7 +127,9 @@ loop(int toplevel, int justonce) * no matter what. */ errflag = 0; + unqueue_signals(); preprompt(); + queue_signals(); if (stophist != 3) hbegin(1); else @@ -218,6 +221,7 @@ loop(int toplevel, int justonce) if (((!interact || sourcelevel) && errflag) || retflag) break; if (isset(SINGLECOMMAND) && toplevel) { + dont_queue_signals(); if (sigtrapped[SIGEXIT]) dotrap(SIGEXIT); exit(lastval); @@ -229,6 +233,7 @@ loop(int toplevel, int justonce) if (!toplevel) zcontext_restore(); popheap(); + unqueue_signals(); if (err) return LOOP_ERROR; diff --git a/Src/input.c b/Src/input.c index 1efabadeb..eb968ea72 100644 --- a/Src/input.c +++ b/Src/input.c @@ -141,16 +141,19 @@ shingetline(void) int c; char buf[BUFSIZ]; char *p; + int q = queue_signal_level(); p = buf; - winch_unblock(); for (;;) { + winch_unblock(); + dont_queue_signals(); do { errno = 0; c = fgetc(bshin); } while (c < 0 && errno == EINTR); if (c < 0 || c == '\n') { winch_block(); + restore_queue_signals(q); if (c == '\n') *p++ = '\n'; if (p > buf) { @@ -167,12 +170,13 @@ shingetline(void) *p++ = c; if (p >= buf + BUFSIZ - 1) { winch_block(); + queue_signals(); line = zrealloc(line, ll + (p - buf) + 1); memcpy(line + ll, buf, p - buf); ll += p - buf; line[ll] = '\0'; p = buf; - winch_unblock(); + unqueue_signals(); } } } @@ -377,6 +381,8 @@ inputline(void) static void inputsetline(char *str, int flags) { + queue_signals(); + if ((inbufflags & INP_FREE) && inbuf) { free(inbuf); } @@ -394,6 +400,8 @@ inputsetline(char *str, int flags) else inbufct = inbufleft; inbufflags = flags; + + unqueue_signals(); } /* diff --git a/Src/loop.c b/Src/loop.c index e4e8e2df8..4def9b652 100644 --- a/Src/loop.c +++ b/Src/loop.c @@ -56,6 +56,10 @@ execfor(Estate state, int do_exec) char *name, *str, *cond = NULL, *advance = NULL; zlong val = 0; LinkList vars = NULL, args = NULL; + int old_simple_pline = simple_pline; + + /* See comments in execwhile() */ + simple_pline = 1; end = state->pc + WC_FOR_SKIP(code); @@ -69,10 +73,12 @@ execfor(Estate state, int do_exec) fprintf(xtrerr, "%s\n", str2); fflush(xtrerr); } - if (!errflag) + if (!errflag) { matheval(str); + } if (errflag) { state->pc = end; + simple_pline = old_simple_pline; return 1; } cond = ecgetstr(state, EC_NODUP, &ctok); @@ -85,12 +91,14 @@ execfor(Estate state, int do_exec) if (!(args = ecgetlist(state, *state->pc++, EC_DUPTOK, &htok))) { state->pc = end; + simple_pline = old_simple_pline; return 0; } if (htok) { execsubst(args); if (errflag) { state->pc = end; + simple_pline = old_simple_pline; return 1; } } @@ -198,6 +206,7 @@ execfor(Estate state, int do_exec) popheap(); cmdpop(); loops--; + simple_pline = old_simple_pline; state->pc = end; return lastval; } @@ -214,6 +223,10 @@ execselect(Estate state, UNUSED(int do_exec)) FILE *inp; size_t more; LinkList args; + int old_simple_pline = simple_pline; + + /* See comments in execwhile() */ + simple_pline = 1; end = state->pc + WC_FOR_SKIP(code); name = ecgetstr(state, EC_NODUP, NULL); @@ -229,18 +242,21 @@ execselect(Estate state, UNUSED(int do_exec)) if (!(args = ecgetlist(state, *state->pc++, EC_DUPTOK, &htok))) { state->pc = end; + simple_pline = old_simple_pline; return 0; } if (htok) { execsubst(args); if (errflag) { state->pc = end; + simple_pline = old_simple_pline; return 1; } } } if (!args || empty(args)) { state->pc = end; + simple_pline = old_simple_pline; return 0; } loops++; @@ -315,6 +331,7 @@ execselect(Estate state, UNUSED(int do_exec)) popheap(); fclose(inp); loops--; + simple_pline = old_simple_pline; state->pc = end; return lastval; } @@ -382,6 +399,7 @@ execwhile(Estate state, UNUSED(int do_exec)) Wordcode end, loop; wordcode code = state->pc[-1]; int olderrexit, oldval, isuntil = (WC_WHILE_TYPE(code) == WC_WHILE_UNTIL); + int old_simple_pline = simple_pline; end = state->pc + WC_WHILE_SKIP(code); olderrexit = noerrexit; @@ -396,8 +414,6 @@ execwhile(Estate state, UNUSED(int do_exec)) /* This is an empty loop. Make sure the signal handler sets the * flags and then just wait for someone hitting ^C. */ - int old_simple_pline = simple_pline; - simple_pline = 1; while (!breaks) @@ -409,7 +425,14 @@ execwhile(Estate state, UNUSED(int do_exec)) for (;;) { state->pc = loop; noerrexit = 1; + + /* In case the test condition is a functional no-op, + * make sure signal handlers recognize ^C to end the loop. */ + simple_pline = 1; + execlist(state, 1, 0); + + simple_pline = old_simple_pline; noerrexit = olderrexit; if (!((lastval == 0) ^ isuntil)) { if (breaks) @@ -421,7 +444,14 @@ execwhile(Estate state, UNUSED(int do_exec)) lastval = oldval; break; } + + /* In case the loop body is also a functional no-op, + * make sure signal handlers recognize ^C as above. */ + simple_pline = 1; + execlist(state, 1, 0); + + simple_pline = old_simple_pline; if (breaks) { breaks--; if (breaks || !contflag) @@ -452,6 +482,10 @@ execrepeat(Estate state, UNUSED(int do_exec)) wordcode code = state->pc[-1]; int count, htok = 0; char *tmp; + int old_simple_pline = simple_pline; + + /* See comments in execwhile() */ + simple_pline = 1; end = state->pc + WC_REPEAT_SKIP(code); @@ -484,6 +518,7 @@ execrepeat(Estate state, UNUSED(int do_exec)) cmdpop(); popheap(); loops--; + simple_pline = old_simple_pline; state->pc = end; return lastval; } diff --git a/Src/parse.c b/Src/parse.c index 09567fde2..1a7416449 100644 --- a/Src/parse.c +++ b/Src/parse.c @@ -456,6 +456,8 @@ init_parse_status(void) void init_parse(void) { + queue_signals(); + if (ecbuf) zfree(ecbuf, eclen); ecbuf = (Wordcode) zalloc((eclen = EC_INIT_SIZE) * sizeof(wordcode)); @@ -466,6 +468,8 @@ init_parse(void) ecnfunc = 0; init_parse_status(); + + unqueue_signals(); } /* Build eprog. */ @@ -488,6 +492,8 @@ bld_eprog(int heap) Eprog ret; int l; + queue_signals(); + ecadd(WCB_END()); ret = heap ? (Eprog) zhalloc(sizeof(*ret)) : (Eprog) zalloc(sizeof(*ret)); @@ -511,6 +517,8 @@ bld_eprog(int heap) zfree(ecbuf, eclen); ecbuf = NULL; + unqueue_signals(); + return ret; } diff --git a/Src/signals.c b/Src/signals.c index 3950ad1a2..697c4c5ec 100644 --- a/Src/signals.c +++ b/Src/signals.c @@ -1207,6 +1207,8 @@ dotrapargs(int sig, int *sigtr, void *sigfn) } } + queue_signals(); /* Any time we manage memory or global state */ + intrap++; *sigtr |= ZSIG_IGNORED; @@ -1244,7 +1246,7 @@ dotrapargs(int sig, int *sigtr, void *sigfn) sfcontext = SFC_SIGNAL; incompfunc = 0; - doshfunc((Shfunc)sigfn, args, 1); + doshfunc((Shfunc)sigfn, args, 1); /* manages signal queueing */ sfcontext = osc; incompfunc= old_incompfunc; freelinklist(args, (FreeFunc) NULL); @@ -1254,7 +1256,7 @@ dotrapargs(int sig, int *sigtr, void *sigfn) trap_state = TRAP_STATE_PRIMED; trapisfunc = isfunc = 0; - execode((Eprog)sigfn, 1, 0, "trap"); + execode((Eprog)sigfn, 1, 0, "trap"); /* manages signal queueing */ } runhookdef(AFTERTRAPHOOK, NULL); @@ -1321,6 +1323,8 @@ dotrapargs(int sig, int *sigtr, void *sigfn) if (*sigtr != ZSIG_IGNORED) *sigtr &= ~ZSIG_IGNORED; intrap--; + + unqueue_signals(); } /* Standard call to execute a trap for a given signal. */ -- cgit 1.4.1