On 16/03/10 07:10, Jim Meyering wrote: > Pádraig Brady wrote: >> +...@opindex --kill-delay > > The long option name makes it sound like this > is merely specifying the delay, when in fact it is > enabling new behavior as well. Did you consider say, > "--kill-after=DELAY" ? > > Whatever its name, saying what happens *without* the option > might clarify: > > Without this option, if the selected signal proves not to be fatal, > @command{timeout} does not kill the @var{command}.
That's better. I'll push the attached patch today sometime. >> + alarm(kill_delay); > > We need one of those pesky space-before paren things ;-) > There's also one missing on a sigemptyset use. Grr :) There are a few of those: for c in *.c; do echo "*******$c*******" cpp -fpreprocessed $c 2>/dev/null | grep '[a-zA-Z0-9](' done *******copy.c******* if (!(sb.st_mode & S_IWUSR) && geteuid() != 0) *******dd.c******* input_offset = MAX(0, offset); *******join.c******* prjoin(seq1.lines[0], seq2.lines[0]); *******pwd.c******* if (stat (wd, &st1) == 0 && stat (".", &st2) == 0 && SAME_INODE(st1, st2)) *******sort.c******* long_options[oi].name, quote(s)); long_options[oi].name, quote("2")); long_options[oi].name, quote(s)); free(fps); free(buffer); free(ord); free(base); free(cur); memmove(&files[i + 1], &files[i + num_merged], memmove(&files[out], &files[in], (nfiles - in) * sizeof *files); *******uptime.c******* printf(_(" \ printf(_("\n")); cheers, Pádraig.
>From dd5f0ee307c4a3c169d22acd400a2c321247ad3b Mon Sep 17 00:00:00 2001 From: =?utf-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com> Date: Mon, 15 Mar 2010 23:03:30 +0000 Subject: [PATCH] timeout: add the --kill-after option Based on a report from Kim Hansen who wanted to send a KILL signal to the monitored command when `timeout` itself received a termination signal. Rather than changing such a signal into a KILL, we provide the more general mechanism of sending the KILL after the specified grace period. * src/timeout.c (cleanup): If a non zero kill delay is specified, (re)set the alarm to that delay, after which a KILL signal will be sent to the process group. (usage): Mention the new option. Separate the description of DURATION since it's now specified in 2 places. Clarify that the duration is an integer. (parse_duration): A new function refactored from main(), since this logic is now called for two parameters. (main): Parse the -k option. * doc/coreutils.texi (timeout invocation): Describe the new --kill-after option and use @display rather than @table to show the duration suffixes. Clarify that a duration of 0 disables the associated timeout. * tests/misc/timeout-parameters: Check invalid --kill-after. * tests/misc/timeout: Check a valid --kill-after works. * NEWS: Mention the new feature. --- NEWS | 4 ++ doc/coreutils.texi | 39 ++++++++++++----------- src/timeout.c | 69 ++++++++++++++++++++++++++++------------- tests/misc/timeout | 6 +++ tests/misc/timeout-parameters | 4 ++ 5 files changed, 82 insertions(+), 40 deletions(-) diff --git a/NEWS b/NEWS index 2a3ca63..60f214b 100644 --- a/NEWS +++ b/NEWS @@ -17,6 +17,10 @@ GNU coreutils NEWS -*- outline -*- join now accepts the --header option, to treat the first line of each file as a header line to be joined and printed unconditionally. + timeout now accepts the --kill-delay option which sends a kill + signal to the monitored command if it's still running the specified + duration after the initial signal was sent. + who: the "+/-" --mesg (-T) indicator of whether a user/tty is accepting messages could be incorrectly listed as "+", when in fact, the user was not accepting messages (mesg no). Before, who would examine only the diff --git a/doc/coreutils.texi b/doc/coreutils.texi index 34ccf5a..e7c09c5 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -15221,31 +15221,25 @@ might find this idea strange at first. still running after the specified time interval. Synopsis: @example -timeout [...@var{option}] @var{number}[smhd] @var{command} [...@var{arg}]@dots{} +timeout [...@var{option}] @var{duration} @var{command} [...@var{arg}]@dots{} @end example -...@cindex time units -...@var{number} is an integer followed by an optional unit; the default -is seconds. The units are: - -...@table @samp -...@item s -seconds -...@item m -minutes -...@item h -hours -...@item d -days -...@end table - @var{command} must not be a special built-in utility (@pxref{Special built-in utilities}). -The program accepts the following option. Also see @ref{Common options}. +The program accepts the following options. Also see @ref{Common options}. Options must precede operands. @table @samp +...@item -k @var{duration} +...@itemx --kill-aft...@var{duration} +...@opindex -k +...@opindex --kill-after +Ensure the monitored @var{command} is killed by also sending a @samp{KILL} +signal, after the specified @var{duration}. Without this option, if the +selected signal proves not to be fatal, @command{timeout} does not kill +the @var{command}. + @item -s @var{signal} @itemx --sign...@var{signal} @opindex -s @@ -15253,9 +15247,18 @@ Options must precede operands. Send this @var{signal} to @var{command} on timeout, rather than the default @samp{TERM} signal. @var{signal} may be a name like @samp{HUP} or a number. Also see @xref{Signal specifications}. - @end table +...@cindex time units +...@var{duration} is an integer followed by an optional unit: +...@display +...@samp{s} for seconds (the default) +...@samp{m} for minutes +...@samp{h} for hours +...@samp{d} for days +...@end display +A duration of 0 disables the associated timeout. + @cindex exit status of @command{timeout} Exit status: diff --git a/src/timeout.c b/src/timeout.c index 10dcbe8..49fc4d8 100644 --- a/src/timeout.c +++ b/src/timeout.c @@ -77,9 +77,11 @@ static int timed_out; static int term_signal = SIGTERM; /* same default as kill command. */ static int monitored_pid; static int sigs_to_ignore[NSIG]; /* so monitor can ignore sigs it resends. */ +static unsigned long kill_after; static struct option const long_options[] = { + {"kill-after", required_argument, NULL, 'k'}, {"signal", required_argument, NULL, 's'}, {NULL, 0, NULL, 0} }; @@ -108,6 +110,13 @@ cleanup (int sig) sigs_to_ignore[sig] = 0; return; } + if (kill_after) + { + /* Start a new timeout after which we'll send SIGKILL. */ + term_signal = SIGKILL; + alarm (kill_after); + kill_after = 0; /* Don't let later signals reset kill alarm. */ + } send_sig (0, sig); if (sig != SIGKILL && sig != SIGCONT) send_sig (0, SIGCONT); @@ -125,20 +134,18 @@ usage (int status) else { printf (_("\ -Usage: %s [OPTION] NUMBER[SUFFIX] COMMAND [ARG]...\n\ +Usage: %s [OPTION] DURATION COMMAND [ARG]...\n\ or: %s [OPTION]\n"), program_name, program_name); fputs (_("\ -Start COMMAND, and kill it if still running after NUMBER seconds.\n\ -SUFFIX may be `s' for seconds (the default), `m' for minutes,\n\ -`h' for hours or `d' for days.\n\ +Start COMMAND, and kill it if still running after DURATION.\n\ \n\ -"), stdout); - - fputs (_("\ Mandatory arguments to long options are mandatory for short options too.\n\ "), stdout); fputs (_("\ + -k, --kill-after=DURATION\n\ + also send a KILL signal if COMMAND is still running\n\ + this long after the initial signal was sent.\n\ -s, --signal=SIGNAL\n\ specify the signal to be sent on timeout.\n\ SIGNAL may be a name like `HUP' or a number.\n\ @@ -146,6 +153,12 @@ Mandatory arguments to long options are mandatory for short options too.\n\ fputs (HELP_OPTION_DESCRIPTION, stdout); fputs (VERSION_OPTION_DESCRIPTION, stdout); + + fputs (_("\n\ +DURATION is an integer with an optional suffix:\n\ +`s' for seconds(the default), `m' for minutes, `h' for hours or `d' for days.\n\ +"), stdout); + fputs (_("\n\ If the command times out, then exit with status 124. Otherwise, exit\n\ with the status of COMMAND. If no signal is specified, send the TERM\n\ @@ -195,11 +208,32 @@ apply_time_suffix (unsigned long *x, char suffix_char) return true; } +static unsigned long +parse_duration (const char* str) +{ + unsigned long duration; + char *ep; + + if (xstrtoul (str, &ep, 10, &duration, NULL) + /* Invalid interval. Note 0 disables timeout */ + || (duration > UINT_MAX) + /* Extra chars after the number and an optional s,m,h,d char. */ + || (*ep && *(ep + 1)) + /* Check any suffix char and update timeout based on the suffix. */ + || !apply_time_suffix (&duration, *ep)) + { + error (0, 0, _("invalid time interval %s"), quote (str)); + usage (EXIT_CANCELED); + } + + return duration; +} + static void install_signal_handlers (int sigterm) { struct sigaction sa; - sigemptyset(&sa.sa_mask); /* Allow concurrent calls to handler */ + sigemptyset (&sa.sa_mask); /* Allow concurrent calls to handler */ sa.sa_handler = cleanup; sa.sa_flags = SA_RESTART; /* restart syscalls (like wait() below) */ @@ -217,7 +251,6 @@ main (int argc, char **argv) unsigned long timeout; char signame[SIG2STR_MAX]; int c; - char *ep; initialize_main (&argc, &argv); set_program_name (argv[0]); @@ -231,10 +264,13 @@ main (int argc, char **argv) parse_long_options (argc, argv, PROGRAM_NAME, PACKAGE_NAME, Version, usage, AUTHORS, (char const *) NULL); - while ((c = getopt_long (argc, argv, "+s:", long_options, NULL)) != -1) + while ((c = getopt_long (argc, argv, "+k:s:", long_options, NULL)) != -1) { switch (c) { + case 'k': + kill_after = parse_duration (optarg); + break; case 's': term_signal = operand2sig (optarg, signame); if (term_signal == -1) @@ -249,18 +285,7 @@ main (int argc, char **argv) if (argc - optind < 2) usage (EXIT_CANCELED); - if (xstrtoul (argv[optind], &ep, 10, &timeout, NULL) - /* Invalid interval. Note 0 disables timeout */ - || (timeout > UINT_MAX) - /* Extra chars after the number and an optional s,m,h,d char. */ - || (*ep && *(ep + 1)) - /* Check any suffix char and update timeout based on the suffix. */ - || !apply_time_suffix (&timeout, *ep)) - { - error (0, 0, _("invalid time interval %s"), quote (argv[optind])); - usage (EXIT_CANCELED); - } - optind++; + timeout = parse_duration (argv[optind++]); argv += optind; diff --git a/tests/misc/timeout b/tests/misc/timeout index 3bd3af3..6c9b02e 100755 --- a/tests/misc/timeout +++ b/tests/misc/timeout @@ -40,6 +40,12 @@ test $? = 2 || fail=1 timeout 1 sleep 10 test $? = 124 || fail=1 +# kill delay. Note once the initial timeout triggers, +# the exit status will be 124 even if the command +# exits on its own accord. +timeout -s0 -k1 1 sleep 10 +test $? = 124 && fail=1 + # Ensure `timeout` is immune to parent's SIGCHLD handler # Use a subshell and an exec to work around a bug in FreeBSD 5.0 /bin/sh. ( diff --git a/tests/misc/timeout-parameters b/tests/misc/timeout-parameters index a55b2d2..6108131 100755 --- a/tests/misc/timeout-parameters +++ b/tests/misc/timeout-parameters @@ -35,6 +35,10 @@ test $? = 125 || fail=1 timeout invalid sleep 0 test $? = 125 || fail=1 +# invalid kill delay +timeout --kill-after=invalid 1 sleep 0 +test $? = 125 || fail=1 + # invalid timeout suffix timeout 42D sleep 0 test $? = 125 || fail=1 -- 1.6.2.5