From 7566bae19715a493f40fd3fae4d69d10089af411 Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Tue, 31 Mar 2009 17:24:49 +0000 Subject: [PATCH] hush: fix wait builtin function old new delta builtin_wait 174 275 +101 sigwaitinfo - 48 +48 __GI_sigwaitinfo - 48 +48 check_and_run_traps 133 169 +36 checkjobs 349 380 +31 hush_main 971 991 +20 static.zero_timespec - 8 +8 run_list 2010 2016 +6 file_get 254 260 +6 static.zero_ts 8 - -8 ------------------------------------------------------------------------------ (add/remove: 3/1 grow/shrink: 6/0 up/down: 304/-8) Total: 296 bytes --- shell/hush.c | 130 +++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 99 insertions(+), 31 deletions(-) diff --git a/shell/hush.c b/shell/hush.c index de3b56d21..af6763517 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -491,13 +491,14 @@ struct globals { unsigned char charmap[256]; char user_input_buf[ENABLE_FEATURE_EDITING ? BUFSIZ : 2]; /* Signal and trap handling */ - char **traps; /* char *traps[NSIG] */ +// unsigned count_SIGCHLD; +// unsigned handled_SIGCHLD; /* which signals have non-DFL handler (even with no traps set)? */ unsigned non_DFL_mask; + char **traps; /* char *traps[NSIG] */ sigset_t blocked_set; sigset_t inherited_set; }; - #define G (*ptr_to_globals) /* Not #defining name to G.name - this quickly gets unwieldy * (too many defines). Also, I actually prefer to see when a variable @@ -829,12 +830,18 @@ static void free_strings(char **strings) * POSIX says pending signal mask is cleared in child - no need to clear it. * Restore blocked signal set to one inherited by shell just prior to exec. * - * Note: as a result, we do not use signal handlers much. The only use - * is to restore terminal pgrp on exit. + * Note: as a result, we do not use signal handlers much. The only uses + * are to count SIGCHLDs [disabled - bug somewhere, + bloat] + * and to restore tty pgrp on signal-induced exit. * * TODO: check/fix wait builtin to be interruptible. */ +//static void SIGCHLD_handler(int sig UNUSED_PARAM) +//{ +// G.count_SIGCHLD++; +//} + /* called once at shell init */ static void init_signal_mask(void) { @@ -863,20 +870,24 @@ static void init_signal_mask(void) mask >>= 1; sig++; } + sigdelset(&G.blocked_set, SIGCHLD); sigprocmask(SIG_SETMASK, &G.blocked_set, &G.inherited_set); } -static void check_and_run_traps(void) +static int check_and_run_traps(int sig) { - static const struct timespec zero_ts = { 0, 0 }; + static const struct timespec zero_timespec = { 0, 0 }; smalluint save_rcode; - int sig; + int last_sig = 0; + if (sig) + goto jump_in; while (1) { - sig = sigtimedwait(&G.blocked_set, NULL, &zero_ts); + sig = sigtimedwait(&G.blocked_set, NULL, &zero_timespec); if (sig <= 0) break; - + jump_in: + last_sig = sig; if (G.traps && G.traps[sig]) { if (G.traps[sig][0]) { /* We have user-defined handler */ @@ -890,7 +901,11 @@ static void check_and_run_traps(void) } /* not a trap: special action */ switch (sig) { +// case SIGCHLD: +// G.count_SIGCHLD++; +// break; case SIGINT: + bb_putchar('\n'); G.flag_SIGINT = 1; break; //TODO @@ -900,6 +915,7 @@ static void check_and_run_traps(void) break; } } + return last_sig; } #if ENABLE_HUSH_JOB @@ -1209,7 +1225,7 @@ static void get_user_input(struct in_str *i) * only after . (^C will work) */ r = read_line_input(prompt_str, G.user_input_buf, BUFSIZ-1, G.line_input_state); /* catch *SIGINT* etc (^C is handled by read_line_input) */ - check_and_run_traps(); + check_and_run_traps(0); } while (r == 0 || G.flag_SIGINT); /* repeat if ^C or SIGINT */ i->eof_flag = (r < 0); if (i->eof_flag) { /* EOF/error detected */ @@ -1223,7 +1239,7 @@ static void get_user_input(struct in_str *i) fflush(stdout); G.user_input_buf[0] = r = fgetc(i->file); /*G.user_input_buf[1] = '\0'; - already is and never changed */ -//do we need check_and_run_traps()? (maybe only if stdin) +//do we need check_and_run_traps(0)? (maybe only if stdin) } while (G.flag_SIGINT); i->eof_flag = (r == EOF); #endif @@ -2329,6 +2345,11 @@ static int checkjobs(struct pipe* fg_pipe) debug_printf_jobs("checkjobs %p\n", fg_pipe); + errno = 0; +// if (G.handled_SIGCHLD == G.count_SIGCHLD) +// /* avoid doing syscall, nothing there anyway */ +// return rcode; + attributes = WUNTRACED; if (fg_pipe == NULL) attributes |= WNOHANG; @@ -2346,10 +2367,21 @@ static int checkjobs(struct pipe* fg_pipe) // + killall -STOP cat wait_more: -// TODO: safe_waitpid? - while ((childpid = waitpid(-1, &status, attributes)) > 0) { + while (1) { int i; - const int dead = WIFEXITED(status) || WIFSIGNALED(status); + int dead; + +// i = G.count_SIGCHLD; + childpid = waitpid(-1, &status, attributes); + if (childpid <= 0) { + if (childpid && errno != ECHILD) + bb_perror_msg("waitpid"); +// else /* Until next SIGCHLD, waitpid's are useless */ +// G.handled_SIGCHLD = i; + break; + } + dead = WIFEXITED(status) || WIFSIGNALED(status); + #if DEBUG_JOBS if (WIFSTOPPED(status)) debug_printf_jobs("pid %d stopped by sig %d (exitcode %d)\n", @@ -2428,10 +2460,6 @@ static int checkjobs(struct pipe* fg_pipe) #endif } /* while (waitpid succeeds)... */ - /* wait found no children or failed */ - - if (childpid && errno != ECHILD) - bb_perror_msg("waitpid"); return rcode; } @@ -3004,7 +3032,7 @@ static int run_list(struct pipe *pi) if (r != -1) { /* we only ran a builtin: rcode is already known * and we don't need to wait for anything. */ - check_and_run_traps(); + check_and_run_traps(0); #if ENABLE_HUSH_LOOPS /* was it "break" or "continue"? */ if (G.flag_break_continue) { @@ -3029,7 +3057,7 @@ static int run_list(struct pipe *pi) /* even bash 3.2 doesn't do that well with nested bg: * try "{ { sleep 10; echo DEEP; } & echo HERE; } &". * I'm NOT treating inner &'s as jobs */ - check_and_run_traps(); + check_and_run_traps(0); #if ENABLE_HUSH_JOB if (G.run_list_level == 1) insert_bg_job(pi); @@ -3040,13 +3068,13 @@ static int run_list(struct pipe *pi) if (G.run_list_level == 1 && G.interactive_fd) { /* waits for completion, then fg's main shell */ rcode = checkjobs_and_fg_shell(pi); - check_and_run_traps(); + check_and_run_traps(0); debug_printf_exec(": checkjobs_and_fg_shell returned %d\n", rcode); } else #endif { /* this one just waits for completion */ rcode = checkjobs(pi); - check_and_run_traps(); + check_and_run_traps(0); debug_printf_exec(": checkjobs returned %d\n", rcode); } } @@ -3668,9 +3696,9 @@ static int process_command_subs(o_string *dest, } debug_printf("done reading from pipe, pclose()ing\n"); - /* This is the step that wait()s for the child. Should be pretty + /* This is the step that waits for the child. Should be pretty * safe, since we just read an EOF from its stdout. We could try - * to do better, by using wait(), and keeping track of background jobs + * to do better, by using waitpid, and keeping track of background jobs * at the same time. That would be a lot of work, and contrary * to the KISS philosophy of this program. */ retcode = fclose(p); @@ -4586,7 +4614,7 @@ int hush_main(int argc, char **argv) // to (inadvertently) close/redirect it } } - init_signal_mask(); + init_signal_mask(); /* note: ensures SIGCHLD is not masked */ debug_printf("G.interactive_fd=%d\n", G.interactive_fd); if (G.interactive_fd) { fcntl(G.interactive_fd, F_SETFD, FD_CLOEXEC); @@ -4617,8 +4645,12 @@ int hush_main(int argc, char **argv) fcntl(G.interactive_fd, F_SETFD, FD_CLOEXEC); } } - init_signal_mask(); + init_signal_mask(); /* note: ensures SIGCHLD is not masked */ #endif + /* POSIX allows shell to re-enable SIGCHLD + * even if it was SIG_IGN on entry */ +// G.count_SIGCHLD++; /* ensure it is != G.handled_SIGCHLD */ + signal(SIGCHLD, SIG_DFL); // SIGCHLD_handler); #if ENABLE_HUSH_INTERACTIVE && !ENABLE_FEATURE_SH_EXTRA_QUIET if (G.interactive_fd) { @@ -5164,12 +5196,48 @@ static int builtin_unset(char **argv) static int builtin_wait(char **argv) { int ret = EXIT_SUCCESS; - int status; - - if (*++argv == NULL) - /* don't care about exit status */ - wait(NULL); + int status, sig; + + if (*++argv == NULL) { + /* Don't care about wait results */ + /* Note 1: must wait until there are no more children */ + /* Note 2: must be interruptible */ + /* Examples: + * $ sleep 3 & sleep 6 & wait + * [1] 30934 sleep 3 + * [2] 30935 sleep 6 + * [1] Done sleep 3 + * [2] Done sleep 6 + * $ sleep 3 & sleep 6 & wait + * [1] 30936 sleep 3 + * [2] 30937 sleep 6 + * [1] Done sleep 3 + * ^C <-- after ~4 sec from keyboard + * $ + */ + sigaddset(&G.blocked_set, SIGCHLD); + sigprocmask(SIG_SETMASK, &G.blocked_set, NULL); + while (1) { + checkjobs(NULL); + if (errno == ECHILD) + break; + /* Wait for SIGCHLD or any other signal of interest */ + /* sigtimedwait with infinite timeout: */ + sig = sigwaitinfo(&G.blocked_set, NULL); + if (sig > 0) { + sig = check_and_run_traps(sig); + if (sig && sig != SIGCHLD) { /* see note 2 */ + ret = 128 + sig; + break; + } + } + } + sigdelset(&G.blocked_set, SIGCHLD); + sigprocmask(SIG_SETMASK, &G.blocked_set, NULL); + return ret; + } + /* This is probably buggy wrt interruptible-ness */ while (*argv) { pid_t pid = bb_strtou(*argv, NULL, 10); if (errno) { -- 2.25.1