From 8b800f8fd193ff408ea92a9ea3eb6584a5ec0e9b Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Tue, 19 Dec 2006 10:35:54 +0000 Subject: document OS may handle negative or zero PID in kill --- ChangeLog | 9 ++++ Doc/Zsh/builtins.yo | 3 ++ Src/jobs.c | 8 ++- Src/signals.c | 148 +++++++++++++++++++++++++++++++++++----------------- 4 files changed, 117 insertions(+), 51 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8e02b7863..30f6518f8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2006-12-19 Peter Stephenson + + * 23067: Doc/Zsh/builtins.yo, Src/jobs.c, Src/signals.c: + queue traps but handle signals when waiting for jobs or processes, + unless TRAPSASYNC is set or the wait builtin is in use, so as + to handle untrapped signals in a timely fashion; document that + negative or zero process IDs after kill may be handled specially + by the OS. + 2006-12-18 Peter Stephenson * 23054, part: Src/jobs.c: error message for "kill -" with diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo index 51c1fd00c..e84084cd4 100644 --- a/Doc/Zsh/builtins.yo +++ b/Doc/Zsh/builtins.yo @@ -692,6 +692,9 @@ tt(SIGIO), assuming they correspond to the same signal number. tt(kill show if the alternative form corresponds to a signal number. For example, under Linux tt(kill -l IO) and tt(kill -l POLL) both output 29, hence tt(kill -IO) and tt(kill -POLL) have the same effect. + +Many systems will allow process IDs to be negative to kill a process +group or zero to kill the current process group. ) findex(let) item(tt(let) var(arg) ...)( diff --git a/Src/jobs.c b/Src/jobs.c index 77dbf51c9..9dbdc8185 100644 --- a/Src/jobs.c +++ b/Src/jobs.c @@ -1141,6 +1141,7 @@ waitforpid(pid_t pid, int wait_cmd) /* child_block() around this loop in case #ifndef WNOHANG */ dont_queue_signals(); child_block(); /* unblocked in signal_suspend() */ + queue_traps(wait_cmd); while (!errflag && (kill(pid, 0) >= 0 || errno != ESRCH)) { if (first) first = 0; @@ -1148,7 +1149,7 @@ waitforpid(pid_t pid, int wait_cmd) kill(pid, SIGCONT); last_signal = -1; - signal_suspend(SIGCHLD, wait_cmd); + signal_suspend(SIGCHLD); if (last_signal != SIGCHLD && wait_cmd) { /* wait command interrupted, but no error: return */ restore_queue_signals(q); @@ -1156,6 +1157,7 @@ waitforpid(pid_t pid, int wait_cmd) } child_block(); } + unqueue_traps(); child_unblock(); restore_queue_signals(q); @@ -1177,6 +1179,7 @@ zwaitjob(int job, int wait_cmd) dont_queue_signals(); child_block(); /* unblocked during signal_suspend() */ + queue_traps(wait_cmd); if (jn->procs || jn->auxprocs) { /* if any forks were done */ jn->stat |= STAT_LOCKED; if (jn->stat & STAT_CHANGED) @@ -1184,7 +1187,7 @@ zwaitjob(int job, int wait_cmd) while (!errflag && jn->stat && !(jn->stat & STAT_DONE) && !(interact && (jn->stat & STAT_STOPPED))) { - signal_suspend(SIGCHLD, wait_cmd); + signal_suspend(SIGCHLD); if (last_signal != SIGCHLD && wait_cmd) { /* builtin wait interrupted by trapped signal */ @@ -1211,6 +1214,7 @@ zwaitjob(int job, int wait_cmd) pipestats[0] = lastval; numpipestats = 1; } + unqueue_traps(); child_unblock(); restore_queue_signals(q); diff --git a/Src/signals.c b/Src/signals.c index 2771c98c2..635a7d341 100644 --- a/Src/signals.c +++ b/Src/signals.c @@ -346,39 +346,10 @@ signal_setmask(sigset_t set) static int suspend_longjmp = 0; static signal_jmp_buf suspend_jmp_buf; #endif - -#if defined(POSIX_SIGNALS) || defined(BSD_SIGNALS) -static void -signal_suspend_setup(sigset_t *set, int sig, int wait_cmd) -{ - if (isset(TRAPSASYNC)) { - sigemptyset(set); - } else { - sigfillset(set); - sigdelset(set, sig); -#ifdef POSIX_SIGNALS - sigdelset(set, SIGHUP); /* still don't know why we add this? */ -#endif - if (wait_cmd) - { - /* - * Using "wait" builtin. We should allow SIGINT and - * execute traps delivered to the shell. - */ - int sigtr; - sigdelset(set, SIGINT); - for (sigtr = 1; sigtr <= NSIG; sigtr++) { - if (sigtrapped[sigtr] & ~ZSIG_IGNORED) - sigdelset(set, sigtr); - } - } - } -} -#endif /**/ int -signal_suspend(int sig, int wait_cmd) +signal_suspend(int sig) { int ret; @@ -388,7 +359,7 @@ signal_suspend(int sig, int wait_cmd) sigset_t oset; #endif /* BROKEN_POSIX_SIGSUSPEND */ - signal_suspend_setup(&set, sig, wait_cmd); + sigemptyset(&set); #ifdef BROKEN_POSIX_SIGSUSPEND sigprocmask(SIG_SETMASK, &set, &oset); pause(); @@ -400,7 +371,7 @@ signal_suspend(int sig, int wait_cmd) # ifdef BSD_SIGNALS sigset_t set; - signal_suspend_setup(&set, sig, wait_cmd); + sigemptyset(&set); ret = sigpause(set); # else # ifdef SYSV_SIGNALS @@ -419,7 +390,7 @@ signal_suspend(int sig, int wait_cmd) # endif /* SYSV_SIGNALS */ # endif /* BSD_SIGNALS */ #endif /* POSIX_SIGNALS */ - + return ret; } @@ -561,18 +532,14 @@ zhandler(int sig) break; case SIGHUP: - if (sigtrapped[SIGHUP]) - dotrap(SIGHUP); - else { + if (!handletrap(SIGHUP)) { stopmsg = 1; zexit(SIGHUP, 1); } break; case SIGINT: - if (sigtrapped[SIGINT]) - dotrap(SIGINT); - else { + if (!handletrap(SIGINT)) { if ((isset(PRIVILEGED) || isset(RESTRICTED)) && isset(INTERACTIVE) && noerrexit < 0) zexit(SIGINT, 1); @@ -587,19 +554,12 @@ zhandler(int sig) #ifdef SIGWINCH case SIGWINCH: adjustwinsize(1); /* check window size and adjust */ - if (sigtrapped[SIGWINCH]) - dotrap(SIGWINCH); + (void) handletrap(SIGWINCH); break; #endif case SIGALRM: - if (sigtrapped[SIGALRM]) { - int tmout; - dotrap(SIGALRM); - - if ((tmout = getiparam("TMOUT"))) - alarm(tmout); /* reset the alarm */ - } else { + if (!handletrap(SIGALRM)) { int idle = ttyidlegetfn(NULL); int tmout = getiparam("TMOUT"); if (idle >= 0 && idle < tmout) @@ -614,7 +574,7 @@ zhandler(int sig) break; default: - dotrap(sig); + (void) handletrap(sig); break; } /* end of switch(sig) */ @@ -1000,6 +960,96 @@ endtrapscope(void) "BUG: still saved traps outside all function scope"); } + +/* + * Decide whether a trap needs handling. + * If so, see if the trap should be run now or queued. + * Return 1 if the trap has been or will be handled. + * This only needs to be called in place of dotrap() in the + * signal handler, since it's only while waiting for children + * to exit that we queue traps. + */ +/**/ +static int +handletrap(int sig) +{ + if (!sigtrapped[sig]) + return 0; + + if (trap_queueing_enabled) + { + /* Code borrowed from signal queueing */ + int temp_rear = ++trap_queue_rear % MAX_QUEUE_SIZE; + + DPUTS(temp_rear == trap_queue_front, "BUG: trap queue full"); + /* If queue is not full... */ + if (temp_rear != trap_queue_front) { + trap_queue_rear = temp_rear; + trap_queue[trap_queue_rear] = sig; + } + return 1; + } + + dotrap(sig); + + if (sig == SIGALRM) + { + int tmout; + /* + * Reset the alarm. + * It seems slightly more natural to do this when the + * trap is run, rather than when it's queued, since + * the user doesn't see the latter. + */ + if ((tmout = getiparam("TMOUT"))) + alarm(tmout); + } + + return 1; +} + + +/* + * Queue traps if they shouldn't be run asynchronously, i.e. + * we're not in the wait builtin and TRAPSASYNC isn't set, when + * waiting for children to exit. + * + * Note that unlike signal queuing this should only be called + * in single matching pairs and can't be nested. It is + * only needed when waiting for a job or process to finish. + * + * There is presumably a race setting this up: we shouldn't be running + * traps between forking a foreground process and this point, either. + */ +/**/ +void +queue_traps(int wait_cmd) +{ + if (!isset(TRAPSASYNC) && !wait_cmd) { + /* + * Traps need to be handled synchronously, so + * enable queueing. + */ + trap_queueing_enabled = 1; + } +} + + +/* + * Disable trap queuing and run the traps. + */ +/**/ +void +unqueue_traps(void) +{ + trap_queueing_enabled = 0; + while (trap_queue_front != trap_queue_rear) { + trap_queue_front = (trap_queue_front + 1) % MAX_QUEUE_SIZE; + (void) handletrap(trap_queue[trap_queue_front]); + } +} + + /* Execute a trap function for a given signal, possibly * with non-standard sigtrapped & siglists values */ -- cgit 1.4.1