From b12553faa8991e11c11f70a81f1d9d44078c7645 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 21 Feb 2011 03:22:20 +0100 Subject: [PATCH] ash: fix ash-signals/signal8 testcase failure function old new delta killcmd 109 224 +115 kill_main 882 910 +28 changepath 194 195 +1 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 3/0 up/down: 144/0) Total: 144 bytes Signed-off-by: Denys Vlasenko --- procps/kill.c | 25 ++++++++-- shell/ash.c | 58 +++++++++++++++++++----- shell/ash_test/ash-signals/sigint1.right | 1 + shell/ash_test/ash-signals/sigint1.tests | 41 +++++++++++++++++ shell/hush_test/hush-misc/sigint1.right | 1 + shell/hush_test/hush-misc/sigint1.tests | 41 +++++++++++++++++ 6 files changed, 152 insertions(+), 15 deletions(-) create mode 100644 shell/ash_test/ash-signals/sigint1.right create mode 100755 shell/ash_test/ash-signals/sigint1.tests create mode 100644 shell/hush_test/hush-misc/sigint1.right create mode 100755 shell/hush_test/hush-misc/sigint1.tests diff --git a/procps/kill.c b/procps/kill.c index b51d44a70..39538016e 100644 --- a/procps/kill.c +++ b/procps/kill.c @@ -206,9 +206,27 @@ int kill_main(int argc, char **argv) /* Looks like they want to do a kill. Do that */ while (arg) { - /* Support shell 'space' trick */ - if (arg[0] == ' ') - arg++; +#if ENABLE_ASH || ENABLE_HUSH + /* + * We need to support shell's "hack formats" of + * " -PRGP_ID" (yes, with a leading space) + * and " PID1 PID2 PID3" (with degenerate case "") + */ + while (*arg != '\0') { + char *end; + if (*arg == ' ') + arg++; + pid = bb_strtoi(arg, &end, 10); + if (errno && (errno != EINVAL || *end != ' ')) { + bb_error_msg("invalid number '%s'", arg); + errors++; + } else if (kill(pid, signo) != 0) { + bb_perror_msg("can't kill pid %d", (int)pid); + errors++; + } + arg = end; /* can only point to ' ' or '\0' now */ + } +#else pid = bb_strtoi(arg, NULL, 10); if (errno) { bb_error_msg("invalid number '%s'", arg); @@ -217,6 +235,7 @@ int kill_main(int argc, char **argv) bb_perror_msg("can't kill pid %d", (int)pid); errors++; } +#endif arg = *++argv; } return errors; diff --git a/shell/ash.c b/shell/ash.c index cccd6dd79..98d2c7c29 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -3783,18 +3783,51 @@ setjobctl(int on) static int FAST_FUNC killcmd(int argc, char **argv) { - int i = 1; if (argv[1] && strcmp(argv[1], "-l") != 0) { + int i = 1; do { if (argv[i][0] == '%') { - struct job *jp = getjob(argv[i], 0); - unsigned pid = jp->ps[0].ps_pid; - /* Enough space for ' -NNN' */ - argv[i] = alloca(sizeof(int)*3 + 3); - /* kill_main has matching code to expect - * leading space. Needed to not confuse - * negative pids with "kill -SIGNAL_NO" syntax */ - sprintf(argv[i], " -%u", pid); + /* + * "kill %N" - job kill + * Converting to pgrp / pid kill + */ + struct job *jp; + char *dst; + int j, n; + + jp = getjob(argv[i], 0); + /* + * In jobs started under job control, we signal + * entire process group by kill -PGRP_ID. + * This happens, f.e., in interactive shell. + * + * Otherwise, we signal each child via + * kill PID1 PID2 PID3. + * Testcases: + * sh -c 'sleep 1|sleep 1 & kill %1' + * sh -c 'true|sleep 2 & sleep 1; kill %1' + * sh -c 'true|sleep 1 & sleep 2; kill %1' + */ + n = jp->nprocs; /* can't be 0 (I hope) */ + if (jp->jobctl) + n = 1; + dst = alloca(n * sizeof(int)*4); + argv[i] = dst; + for (j = 0; j < n; j++) { + struct procstat *ps = &jp->ps[j]; + /* Skip non-running and not-stopped members + * (i.e. dead members) of the job + */ + if (ps->ps_status != -1 && !WIFSTOPPED(ps->ps_status)) + continue; + /* + * kill_main has matching code to expect + * leading space. Needed to not confuse + * negative pids with "kill -SIGNAL_NO" syntax + */ + dst += sprintf(dst, jp->jobctl ? " -%u" : " %u", (int)ps->ps_pid); + } + *dst = '\0'; } } while (argv[++i]); } @@ -4227,8 +4260,9 @@ waitcmd(int argc UNUSED_PARAM, char **argv) break; job = job->prev_job; } - } else + } else { job = getjob(*argv, 0); + } /* loop until process terminated or stopped */ while (job->state == JOBRUNNING) blocking_wait_with_raise_on_sig(); @@ -4724,7 +4758,7 @@ forkchild(struct job *jp, union node *n, int mode) #if JOBS /* do job control only in root shell */ doing_jobctl = 0; - if (mode != FORK_NOJOB && jp->jobctl && !oldlvl) { + if (mode != FORK_NOJOB && jp->jobctl && oldlvl == 0) { pid_t pgrp; if (jp->nprocs == 0) @@ -4750,7 +4784,7 @@ forkchild(struct job *jp, union node *n, int mode) ash_msg_and_raise_error("can't open '%s'", bb_dev_null); } } - if (!oldlvl) { + if (oldlvl == 0) { if (iflag) { /* why if iflag only? */ setsignal(SIGINT); setsignal(SIGTERM); diff --git a/shell/ash_test/ash-signals/sigint1.right b/shell/ash_test/ash-signals/sigint1.right new file mode 100644 index 000000000..a9094b056 --- /dev/null +++ b/shell/ash_test/ash-signals/sigint1.right @@ -0,0 +1 @@ +Sending SIGINT to main shell PID diff --git a/shell/ash_test/ash-signals/sigint1.tests b/shell/ash_test/ash-signals/sigint1.tests new file mode 100755 index 000000000..20e45d940 --- /dev/null +++ b/shell/ash_test/ash-signals/sigint1.tests @@ -0,0 +1,41 @@ +# What should happen if non-interactive shell gets SIGINT? + +(sleep 1; echo Sending SIGINT to main shell PID; exec kill -INT $$) & + +# We create a child which exits with 0 even on SIGINT +# (This is truly necessary only if SIGINT is generated by ^C, +# in this testcase even bare "sleep 2" would do because +# we don't send SIGINT _to_ the_ child_...) +$THIS_SH -c 'trap "exit 0" SIGINT; sleep 2' + +# In one second, we (main shell) get SIGINT here. +# The question is whether we should, or should not, exit. + +# bash will not stop here. It will execute next command(s). + +# The rationale for this is described here: +# http://www.cons.org/cracauer/sigint.html +# +# Basically, bash will not exit on SIGINT immediately if it waits +# for a child. It will wait for the child to exit. +# If child exits NOT by dying on SIGINT, then bash will not exit. +# +# The idea is that the following script: +# | emacs file.txt +# | more cmds +# User may use ^C to interrupt editor's ops like search. But then +# emacs exits normally. User expects that script doesn't stop. +# +# This is a nice idea, but detecting "did process really exit +# with SIGINT?" is racy. Consider: +# | bash -c 'while true; do /bin/true; done' +# When ^C is pressed while bash waits for /bin/true to exit, +# it may happen that /bin/true exits with exitcode 0 before +# ^C is delivered to it as SIGINT. bash will see SIGINT, then +# it will see that child exited with 0, and bash will NOT EXIT. + +# Therefore we do not implement bash behavior. +# I'd say that emacs need to put itself into a separate pgrp +# to isolate shell from getting stray SIGINTs from ^C. + +echo Next command after SIGINT was executed diff --git a/shell/hush_test/hush-misc/sigint1.right b/shell/hush_test/hush-misc/sigint1.right new file mode 100644 index 000000000..a9094b056 --- /dev/null +++ b/shell/hush_test/hush-misc/sigint1.right @@ -0,0 +1 @@ +Sending SIGINT to main shell PID diff --git a/shell/hush_test/hush-misc/sigint1.tests b/shell/hush_test/hush-misc/sigint1.tests new file mode 100755 index 000000000..20e45d940 --- /dev/null +++ b/shell/hush_test/hush-misc/sigint1.tests @@ -0,0 +1,41 @@ +# What should happen if non-interactive shell gets SIGINT? + +(sleep 1; echo Sending SIGINT to main shell PID; exec kill -INT $$) & + +# We create a child which exits with 0 even on SIGINT +# (This is truly necessary only if SIGINT is generated by ^C, +# in this testcase even bare "sleep 2" would do because +# we don't send SIGINT _to_ the_ child_...) +$THIS_SH -c 'trap "exit 0" SIGINT; sleep 2' + +# In one second, we (main shell) get SIGINT here. +# The question is whether we should, or should not, exit. + +# bash will not stop here. It will execute next command(s). + +# The rationale for this is described here: +# http://www.cons.org/cracauer/sigint.html +# +# Basically, bash will not exit on SIGINT immediately if it waits +# for a child. It will wait for the child to exit. +# If child exits NOT by dying on SIGINT, then bash will not exit. +# +# The idea is that the following script: +# | emacs file.txt +# | more cmds +# User may use ^C to interrupt editor's ops like search. But then +# emacs exits normally. User expects that script doesn't stop. +# +# This is a nice idea, but detecting "did process really exit +# with SIGINT?" is racy. Consider: +# | bash -c 'while true; do /bin/true; done' +# When ^C is pressed while bash waits for /bin/true to exit, +# it may happen that /bin/true exits with exitcode 0 before +# ^C is delivered to it as SIGINT. bash will see SIGINT, then +# it will see that child exited with 0, and bash will NOT EXIT. + +# Therefore we do not implement bash behavior. +# I'd say that emacs need to put itself into a separate pgrp +# to isolate shell from getting stray SIGINTs from ^C. + +echo Next command after SIGINT was executed -- 2.25.1