From 991a1da14806eefd1c6fc8fc1c0c3d2b90af6f24 Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Sun, 10 Feb 2008 19:02:53 +0000 Subject: [PATCH] ash: fix "orwell bug" 1984. Testcase: trap_handler() { echo trap } trap trap_handler USR1 sleep 3600 & while true; do wait; done --- shell/ash.c | 103 +++++++++++++++++++++------------------------- shell/ash_doc.txt | 31 ++++++++++++++ 2 files changed, 79 insertions(+), 55 deletions(-) create mode 100644 shell/ash_doc.txt diff --git a/shell/ash.c b/shell/ash.c index 8ff5f4c31..a877b5bab 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -164,7 +164,16 @@ struct globals_misc { char *arg0; /* value of $0 */ struct jmploc *exception_handler; - int exception; + +// disabled by vda: cannot understand how it was supposed to work - +// cannot fix bugs. That's why you have to explain your non-trivial designs! +// /* do we generate EXSIG events */ +// int exsig; /* counter */ + volatile int suppressint; /* counter */ + volatile /*sig_atomic_t*/ smallint intpending; /* 1 = got SIGINT */ + /* last pending signal */ + volatile /*sig_atomic_t*/ smallint pendingsig; + smallint exception; /* kind of exception (0..5) */ /* exceptions */ #define EXINT 0 /* SIGINT received */ #define EXERROR 1 /* a generic error */ @@ -172,12 +181,6 @@ struct globals_misc { #define EXEXEC 3 /* command execution failed */ #define EXEXIT 4 /* exit the shell */ #define EXSIG 5 /* trapped signal in wait(1) */ - volatile int suppressint; - volatile sig_atomic_t intpending; - /* do we generate EXSIG events */ - int exsig; - /* last pending signal */ - volatile sig_atomic_t pendingsig; /* trap handler commands */ char *trap[NSIG]; @@ -211,7 +214,7 @@ static struct globals_misc *const ptr_to_globals_misc __attribute__ ((section (" #define exception (G_misc.exception ) #define suppressint (G_misc.suppressint ) #define intpending (G_misc.intpending ) -#define exsig (G_misc.exsig ) +//#define exsig (G_misc.exsig ) #define pendingsig (G_misc.pendingsig ) #define trap (G_misc.trap ) #define isloginsh (G_misc.isloginsh) @@ -281,6 +284,7 @@ raise_interrupt(void) i = EXSIG; if (gotsig[SIGINT - 1] && !trap[SIGINT]) { if (!(rootshell && iflag)) { + /* Kill ourself with SIGINT */ signal(SIGINT, SIG_DFL); raise(SIGINT); } @@ -333,15 +337,6 @@ force_int_on(void) raise_interrupt(); \ } while (0) -#define EXSIGON \ - do { \ - exsig++; \ - xbarrier(); \ - if (pendingsig) \ - raise_exception(EXSIG); \ - } while (0) -/* EXSIG is turned off by evalbltin(). */ - /* * Ignore a signal. Only one usage site - in forkchild() */ @@ -363,10 +358,10 @@ onsig(int signo) gotsig[signo - 1] = 1; pendingsig = signo; - if (exsig || (signo == SIGINT && !trap[SIGINT])) { + if ( /* exsig || */ (signo == SIGINT && !trap[SIGINT])) { if (!suppressint) { pendingsig = 0; - raise_interrupt(); + raise_interrupt(); /* does not return */ } intpending = 1; } @@ -3256,12 +3251,11 @@ setsignal(int signo) struct sigaction act; t = trap[signo]; + action = S_IGN; if (t == NULL) action = S_DFL; else if (*t != '\0') action = S_CATCH; - else - action = S_IGN; if (rootshell && action == S_DFL) { switch (signo) { case SIGINT: @@ -3294,7 +3288,7 @@ setsignal(int signo) /* * current setting unknown */ - if (sigaction(signo, 0, &act) == -1) { + if (sigaction(signo, NULL, &act) == -1) { /* * Pretend it worked; maybe we should give a warning * here, but other shells don't. We don't alter @@ -3302,19 +3296,19 @@ setsignal(int signo) */ return; } + tsig = S_RESET; /* force to be set */ if (act.sa_handler == SIG_IGN) { + tsig = S_HARD_IGN; if (mflag && (signo == SIGTSTP || signo == SIGTTIN || signo == SIGTTOU) ) { tsig = S_IGN; /* don't hard ignore these */ - } else - tsig = S_HARD_IGN; - } else { - tsig = S_RESET; /* force to be set */ + } } } if (tsig == S_HARD_IGN || tsig == action) return; + act.sa_handler = SIG_DFL; switch (action) { case S_CATCH: act.sa_handler = onsig; @@ -3322,13 +3316,11 @@ setsignal(int signo) case S_IGN: act.sa_handler = SIG_IGN; break; - default: - act.sa_handler = SIG_DFL; } *t = action; act.sa_flags = 0; sigfillset(&act.sa_mask); - sigaction(signo, &act, 0); + sigaction(signo, &act, NULL); } /* mode flags for set_curjob */ @@ -3763,7 +3755,8 @@ waitproc(int wait_flags, int *status) if (jobctl) wait_flags |= WUNTRACED; #endif - return waitpid(-1, status, wait_flags); // safe_waitpid? + /* NB: _not_ safe_waitpid, we need to detect EINTR */ + return waitpid(-1, status, wait_flags); } /* @@ -3781,8 +3774,14 @@ dowait(int wait_flags, struct job *job) TRACE(("dowait(%d) called\n", wait_flags)); pid = waitproc(wait_flags, &status); TRACE(("wait returns pid=%d, status=%d\n", pid, status)); - if (pid <= 0) + if (pid <= 0) { + /* If we were doing blocking wait and (probably) got EINTR, + * check for pending sigs received while waiting. + * (NB: can be moved into callers if needed) */ + if (wait_flags == DOWAIT_BLOCK && pendingsig) + raise_exception(EXSIG); return pid; + } INT_OFF; thisjob = NULL; for (jp = curjob; jp; jp = jp->prev_job) { @@ -4004,7 +4003,10 @@ waitcmd(int argc, char **argv) int retval; struct job *jp; - EXSIGON; +// exsig++; +// xbarrier(); + if (pendingsig) + raise_exception(EXSIG); nextopt(nullstr); retval = 0; @@ -4015,10 +4017,8 @@ waitcmd(int argc, char **argv) for (;;) { jp = curjob; while (1) { - if (!jp) { - /* no running procs */ - goto out; - } + if (!jp) /* no running procs */ + goto ret; if (jp->state == JOBRUNNING) break; jp->waited = 1; @@ -4051,7 +4051,7 @@ waitcmd(int argc, char **argv) ; } while (*++argv); - out: + ret: return retval; } @@ -4450,7 +4450,7 @@ clear_traps(void) char **tp; for (tp = trap; tp < &trap[NSIG]; tp++) { - if (*tp && **tp) { /* trap not NULL or SIG_IGN */ + if (*tp && **tp) { /* trap not NULL or "" (SIG_IGN) */ INT_OFF; free(*tp); *tp = NULL; @@ -4613,8 +4613,8 @@ waitforjob(struct job *jp) * intuit from the subprocess exit status whether a SIGINT * occurred, and if so interrupt ourselves. Yuck. - mycroft */ - if (jp->sigint) - raise(SIGINT); + if (jp->sigint) /* TODO: do the same with all signals */ + raise(SIGINT); /* ... by raise(jp->sig) instead? */ } if (jp->state == JOBDONE) #endif @@ -6268,7 +6268,7 @@ expmeta(char *enddir, char *name) p++; if (*p == '.') matchdot++; - while (! intpending && (dp = readdir(dirp)) != NULL) { + while (!intpending && (dp = readdir(dirp)) != NULL) { if (dp->d_name[0] == '.' && ! matchdot) continue; if (pmatch(start, dp->d_name)) { @@ -7437,7 +7437,7 @@ dotrap(void) char *q; int i; int savestatus; - int skip = 0; + int skip; savestatus = exitstatus; pendingsig = 0; @@ -7454,10 +7454,10 @@ dotrap(void) skip = evalstring(p, SKIPEVAL); exitstatus = savestatus; if (skip) - break; + return skip; } - return skip; + return 0; } /* forward declarations - evaluation is fairly recursive business... */ @@ -8434,22 +8434,15 @@ evalcommand(union node *cmd, int flags) } if (evalbltin(cmdentry.u.cmd, argc, argv)) { int exit_status; - int i, j; - - i = exception; + int i = exception; if (i == EXEXIT) goto raise; - exit_status = 2; - j = 0; if (i == EXINT) - j = SIGINT; + exit_status = 128 + SIGINT; if (i == EXSIG) - j = pendingsig; - if (j) - exit_status = j + 128; + exit_status = 128 + pendingsig; exitstatus = exit_status; - if (i == EXINT || spclbltin > 0) { raise: longjmp(exception_handler->loc, 1); @@ -8499,7 +8492,7 @@ evalbltin(const struct builtincmd *cmd, int argc, char **argv) exitstatus |= ferror(stdout); clearerr(stdout); commandname = savecmdname; - exsig = 0; +// exsig = 0; exception_handler = savehandler; return i; diff --git a/shell/ash_doc.txt b/shell/ash_doc.txt new file mode 100644 index 000000000..28c574841 --- /dev/null +++ b/shell/ash_doc.txt @@ -0,0 +1,31 @@ + Wait + signals + +We had some bugs here which are hard to test in testsuite. + +Bug 1280 (http://busybox.net/bugs/view.php?id=1280): +was misbehaving in interactive ash. Correct behavior: + +$ sleep 20 & +$ wait +^C +$ wait +^C +$ wait +^C +... + +Bug 1984 (http://busybox.net/bugs/view.php?id=1984): +traps were not triggering: + +trap_handler_usr () { + echo trap usr +} +trap_handler_int () { + echo trap int +} +trap trap_handler_usr USR1 +trap trap_handler_int INT +sleep 3600 & +echo "Please do: kill -USR1 $$" +echo "or: kill -INT $$" +while true; do wait; echo wait interrupted; done