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

Reply via email to