Re: What signal should timeout send when it gets a SIGTERM?

[Jim Meyering]

Pádraig Brady wrote:
> Subject: [PATCH] timeout: add the --kill-delay 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.
...
> diff --git a/doc/coreutils.texi b/doc/coreutils.texi
...
>  @table @samp
> address@hidden -k @var{duration}
> address@hidden address@hidden
> address@hidden -k
> address@hidden --kill-delay
> +Ensure the monitored @var{command} is killed by also sending a @samp{KILL}
> +signal, after the specified @var{duration}.

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}.

...
> diff --git a/src/timeout.c b/src/timeout.c
> index 8e47327..11709d7 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_delay;
>
>  static struct option const long_options[] =
>  {
> +  {"kill-delay", required_argument, NULL, 'k'},
>    {"signal", required_argument, NULL, 's'},
>    {NULL, 0, NULL, 0}
>  };
> @@ -108,12 +110,11 @@ cleanup (int sig)
>            sigs_to_ignore[sig] = 0;
>            return;
>          }
> -      if (sig == SIGTERM && term_signal == SIGKILL)
> +      if (kill_delay)
>          {
> -          /* If the user has specified SIGKILL, then in the case
> -             where the monitored doesn't respond to the TERM, don't
> -             wait for the original timeout before sending the KILL.  */
> -          alarm(1); /* XXX: Hopefully the process can cleanup in time.  */
> +          /* Start a new timeout after which we'll send SIGKILL.  */
> +          term_signal = SIGKILL;
> +          alarm(kill_delay);

We need one of those pesky space-before paren things ;-)
There's also one missing on a sigemptyset use.

...
>        fputs (_("\
> +  -k, --kill-delay=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\
> @@ -153,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 which may be optionally followed by a suffix:\n\

"which" is often better written as "that", but here we can make
this more concise without sacrificing readability.
How about this?

  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\