ash: fix "orwell bug" 1984. Testcase:

trap_handler() {
        echo trap
    }
    trap trap_handler USR1
    sleep 3600 &
    while true; do wait; done
This commit is contained in:
Denis Vlasenko 2008-02-10 19:02:53 +00:00
parent 0ef240d979
commit 991a1da148
2 changed files with 79 additions and 55 deletions

View File

@ -164,7 +164,16 @@ struct globals_misc {
char *arg0; /* value of $0 */ char *arg0; /* value of $0 */
struct jmploc *exception_handler; 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 */ /* exceptions */
#define EXINT 0 /* SIGINT received */ #define EXINT 0 /* SIGINT received */
#define EXERROR 1 /* a generic error */ #define EXERROR 1 /* a generic error */
@ -172,12 +181,6 @@ struct globals_misc {
#define EXEXEC 3 /* command execution failed */ #define EXEXEC 3 /* command execution failed */
#define EXEXIT 4 /* exit the shell */ #define EXEXIT 4 /* exit the shell */
#define EXSIG 5 /* trapped signal in wait(1) */ #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 */ /* trap handler commands */
char *trap[NSIG]; char *trap[NSIG];
@ -211,7 +214,7 @@ static struct globals_misc *const ptr_to_globals_misc __attribute__ ((section ("
#define exception (G_misc.exception ) #define exception (G_misc.exception )
#define suppressint (G_misc.suppressint ) #define suppressint (G_misc.suppressint )
#define intpending (G_misc.intpending ) #define intpending (G_misc.intpending )
#define exsig (G_misc.exsig ) //#define exsig (G_misc.exsig )
#define pendingsig (G_misc.pendingsig ) #define pendingsig (G_misc.pendingsig )
#define trap (G_misc.trap ) #define trap (G_misc.trap )
#define isloginsh (G_misc.isloginsh) #define isloginsh (G_misc.isloginsh)
@ -281,6 +284,7 @@ raise_interrupt(void)
i = EXSIG; i = EXSIG;
if (gotsig[SIGINT - 1] && !trap[SIGINT]) { if (gotsig[SIGINT - 1] && !trap[SIGINT]) {
if (!(rootshell && iflag)) { if (!(rootshell && iflag)) {
/* Kill ourself with SIGINT */
signal(SIGINT, SIG_DFL); signal(SIGINT, SIG_DFL);
raise(SIGINT); raise(SIGINT);
} }
@ -333,15 +337,6 @@ force_int_on(void)
raise_interrupt(); \ raise_interrupt(); \
} while (0) } 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() * Ignore a signal. Only one usage site - in forkchild()
*/ */
@ -363,10 +358,10 @@ onsig(int signo)
gotsig[signo - 1] = 1; gotsig[signo - 1] = 1;
pendingsig = signo; pendingsig = signo;
if (exsig || (signo == SIGINT && !trap[SIGINT])) { if ( /* exsig || */ (signo == SIGINT && !trap[SIGINT])) {
if (!suppressint) { if (!suppressint) {
pendingsig = 0; pendingsig = 0;
raise_interrupt(); raise_interrupt(); /* does not return */
} }
intpending = 1; intpending = 1;
} }
@ -3256,12 +3251,11 @@ setsignal(int signo)
struct sigaction act; struct sigaction act;
t = trap[signo]; t = trap[signo];
action = S_IGN;
if (t == NULL) if (t == NULL)
action = S_DFL; action = S_DFL;
else if (*t != '\0') else if (*t != '\0')
action = S_CATCH; action = S_CATCH;
else
action = S_IGN;
if (rootshell && action == S_DFL) { if (rootshell && action == S_DFL) {
switch (signo) { switch (signo) {
case SIGINT: case SIGINT:
@ -3294,7 +3288,7 @@ setsignal(int signo)
/* /*
* current setting unknown * current setting unknown
*/ */
if (sigaction(signo, 0, &act) == -1) { if (sigaction(signo, NULL, &act) == -1) {
/* /*
* Pretend it worked; maybe we should give a warning * Pretend it worked; maybe we should give a warning
* here, but other shells don't. We don't alter * here, but other shells don't. We don't alter
@ -3302,19 +3296,19 @@ setsignal(int signo)
*/ */
return; return;
} }
tsig = S_RESET; /* force to be set */
if (act.sa_handler == SIG_IGN) { if (act.sa_handler == SIG_IGN) {
tsig = S_HARD_IGN;
if (mflag if (mflag
&& (signo == SIGTSTP || signo == SIGTTIN || signo == SIGTTOU) && (signo == SIGTSTP || signo == SIGTTIN || signo == SIGTTOU)
) { ) {
tsig = S_IGN; /* don't hard ignore these */ 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) if (tsig == S_HARD_IGN || tsig == action)
return; return;
act.sa_handler = SIG_DFL;
switch (action) { switch (action) {
case S_CATCH: case S_CATCH:
act.sa_handler = onsig; act.sa_handler = onsig;
@ -3322,13 +3316,11 @@ setsignal(int signo)
case S_IGN: case S_IGN:
act.sa_handler = SIG_IGN; act.sa_handler = SIG_IGN;
break; break;
default:
act.sa_handler = SIG_DFL;
} }
*t = action; *t = action;
act.sa_flags = 0; act.sa_flags = 0;
sigfillset(&act.sa_mask); sigfillset(&act.sa_mask);
sigaction(signo, &act, 0); sigaction(signo, &act, NULL);
} }
/* mode flags for set_curjob */ /* mode flags for set_curjob */
@ -3763,7 +3755,8 @@ waitproc(int wait_flags, int *status)
if (jobctl) if (jobctl)
wait_flags |= WUNTRACED; wait_flags |= WUNTRACED;
#endif #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)); TRACE(("dowait(%d) called\n", wait_flags));
pid = waitproc(wait_flags, &status); pid = waitproc(wait_flags, &status);
TRACE(("wait returns pid=%d, status=%d\n", pid, 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; return pid;
}
INT_OFF; INT_OFF;
thisjob = NULL; thisjob = NULL;
for (jp = curjob; jp; jp = jp->prev_job) { for (jp = curjob; jp; jp = jp->prev_job) {
@ -4004,7 +4003,10 @@ waitcmd(int argc, char **argv)
int retval; int retval;
struct job *jp; struct job *jp;
EXSIGON; // exsig++;
// xbarrier();
if (pendingsig)
raise_exception(EXSIG);
nextopt(nullstr); nextopt(nullstr);
retval = 0; retval = 0;
@ -4015,10 +4017,8 @@ waitcmd(int argc, char **argv)
for (;;) { for (;;) {
jp = curjob; jp = curjob;
while (1) { while (1) {
if (!jp) { if (!jp) /* no running procs */
/* no running procs */ goto ret;
goto out;
}
if (jp->state == JOBRUNNING) if (jp->state == JOBRUNNING)
break; break;
jp->waited = 1; jp->waited = 1;
@ -4051,7 +4051,7 @@ waitcmd(int argc, char **argv)
; ;
} while (*++argv); } while (*++argv);
out: ret:
return retval; return retval;
} }
@ -4450,7 +4450,7 @@ clear_traps(void)
char **tp; char **tp;
for (tp = trap; tp < &trap[NSIG]; 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; INT_OFF;
free(*tp); free(*tp);
*tp = NULL; *tp = NULL;
@ -4613,8 +4613,8 @@ waitforjob(struct job *jp)
* intuit from the subprocess exit status whether a SIGINT * intuit from the subprocess exit status whether a SIGINT
* occurred, and if so interrupt ourselves. Yuck. - mycroft * occurred, and if so interrupt ourselves. Yuck. - mycroft
*/ */
if (jp->sigint) if (jp->sigint) /* TODO: do the same with all signals */
raise(SIGINT); raise(SIGINT); /* ... by raise(jp->sig) instead? */
} }
if (jp->state == JOBDONE) if (jp->state == JOBDONE)
#endif #endif
@ -6268,7 +6268,7 @@ expmeta(char *enddir, char *name)
p++; p++;
if (*p == '.') if (*p == '.')
matchdot++; matchdot++;
while (! intpending && (dp = readdir(dirp)) != NULL) { while (!intpending && (dp = readdir(dirp)) != NULL) {
if (dp->d_name[0] == '.' && ! matchdot) if (dp->d_name[0] == '.' && ! matchdot)
continue; continue;
if (pmatch(start, dp->d_name)) { if (pmatch(start, dp->d_name)) {
@ -7437,7 +7437,7 @@ dotrap(void)
char *q; char *q;
int i; int i;
int savestatus; int savestatus;
int skip = 0; int skip;
savestatus = exitstatus; savestatus = exitstatus;
pendingsig = 0; pendingsig = 0;
@ -7454,10 +7454,10 @@ dotrap(void)
skip = evalstring(p, SKIPEVAL); skip = evalstring(p, SKIPEVAL);
exitstatus = savestatus; exitstatus = savestatus;
if (skip) if (skip)
break; return skip;
} }
return skip; return 0;
} }
/* forward declarations - evaluation is fairly recursive business... */ /* forward declarations - evaluation is fairly recursive business... */
@ -8434,22 +8434,15 @@ evalcommand(union node *cmd, int flags)
} }
if (evalbltin(cmdentry.u.cmd, argc, argv)) { if (evalbltin(cmdentry.u.cmd, argc, argv)) {
int exit_status; int exit_status;
int i, j; int i = exception;
i = exception;
if (i == EXEXIT) if (i == EXEXIT)
goto raise; goto raise;
exit_status = 2; exit_status = 2;
j = 0;
if (i == EXINT) if (i == EXINT)
j = SIGINT; exit_status = 128 + SIGINT;
if (i == EXSIG) if (i == EXSIG)
j = pendingsig; exit_status = 128 + pendingsig;
if (j)
exit_status = j + 128;
exitstatus = exit_status; exitstatus = exit_status;
if (i == EXINT || spclbltin > 0) { if (i == EXINT || spclbltin > 0) {
raise: raise:
longjmp(exception_handler->loc, 1); longjmp(exception_handler->loc, 1);
@ -8499,7 +8492,7 @@ evalbltin(const struct builtincmd *cmd, int argc, char **argv)
exitstatus |= ferror(stdout); exitstatus |= ferror(stdout);
clearerr(stdout); clearerr(stdout);
commandname = savecmdname; commandname = savecmdname;
exsig = 0; // exsig = 0;
exception_handler = savehandler; exception_handler = savehandler;
return i; return i;

31
shell/ash_doc.txt Normal file
View File

@ -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