Subject: Re: bug#9101: timeout should use setitimer if available

From: Pádraig Brady <[email-masked]>
Date: Mon, 18 Jul 2011 11:01:06 +0100

On 16/07/11 20:28, Paul Eggert wrote:
> setitimer has nanosecond resolution, which is better than the
> one-second resolution that 'alarm' has.  timeout should use
> setitimer if available, to take advantage of this.  On 64-bit
> hosts, this has the additional advantage of increasing the
> upper bound for timeouts from 2**31 seconds to 2**63 seconds
> (about 68 years to about 292 billion years, which should be
> long enough for most practical purposes :-).

I'll apply this soon.

cheers,
Pádraig.
>From 057796b81c0de662700a801d49426e9e56e5bc44 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Mon, 18 Jul 2011 10:49:17 +0100
Subject: [PATCH] timeout: support sub-second timeouts

* src/timeout.c (settimeout): A new function to convert
from a floating point duration and all alarm() or
setitimer() if that's available.
(parse_duration): Return a double rather than unsigned int.
(usage): Mention floating point supported.
(main): Pass the double to settimeout() rather than
calling alarm() directly with the parsed int.
(cleanup): Likewise.
* doc/coreutils.texi (timeout invocation): Say floating point timeouts
now supported, and mention the caveat with resolution.
* m4/jm-macros.m4: Check for setitimer.
* tests/misc/timeout-parameters: Add a test for the nanosecond
to microsecond conversion boundary.
* NEWS: Mention the improvement.
---
 NEWS                          |    2 +
 doc/coreutils.texi            |    4 ++-
 m4/jm-macros.m4               |    5 ++-
 src/timeout.c                 |   65 +++++++++++++++++++++++++++++------------
 tests/misc/timeout-parameters |    3 ++
 5 files changed, 58 insertions(+), 21 deletions(-)

diff --git a/NEWS b/NEWS
index c382200..2119f64 100644
--- a/NEWS
+++ b/NEWS
@@ -50,6 +50,8 @@ GNU coreutils NEWS                                    -*- outline -*-
 
   stat -f now recognizes the GPFS, MQUEUE and PSTOREFS file system types.
 
+  timeout allows sub-second timeouts where supported.
+
 ** Build-related
 
   Changes inherited from gnulib address a build failure on HP-UX 11.11
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 424446c..b406a3c 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -15694,7 +15694,7 @@ or a number. Also see @xref{Signal specifications}.
 @end table
 
 @cindex time units
-@var{duration} is an integer followed by an optional unit:
+@var{duration} is a floating point number followed by an optional unit:
 @display
 @samp{s} for seconds (the default)
 @samp{m} for minutes
@@ -15702,6 +15702,8 @@ or a number. Also see @xref{Signal specifications}.
 @samp{d} for days
 @end display
 A duration of 0 disables the associated timeout.
+Note that the actual timeout duration is dependent on system conditions,
+which should be especially considered when specifying sub-second timeouts.
 
 @cindex exit status of @command{timeout}
 Exit status:
diff --git a/m4/jm-macros.m4 b/m4/jm-macros.m4
index 9bb6fa4..fa75708 100644
--- a/m4/jm-macros.m4
+++ b/m4/jm-macros.m4
@@ -66,7 +66,10 @@ AC_DEFUN([coreutils_MACROS],
   # Used by sort.c.
   AC_CHECK_FUNCS_ONCE([nl_langinfo])
   # Used by timeout.c
-  AC_CHECK_FUNCS_ONCE([setrlimit])
+  AC_CHECK_FUNCS_ONCE( \
+    setrlimit \
+    setitimer \
+  )
 
   # Used by tail.c.
   AC_CHECK_FUNCS([inotify_init],
diff --git a/src/timeout.c b/src/timeout.c
index ccb4f85..bbbe8c3 100644
--- a/src/timeout.c
+++ b/src/timeout.c
@@ -78,7 +78,7 @@ 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 double kill_after;
 static bool foreground;            /* whether to use another program group.  */
 
 /* for long options with no corresponding short option, use enum */
@@ -97,6 +97,45 @@ static struct option const long_options[] =
   {NULL, 0, NULL, 0}
 };
 
+/* Start the timeout after which we'll receive a SIGALRM.
+   Round DURATION up to the next representable value.
+   Treat out-of-range values as if they were maximal,
+   as that's more useful in practice than reporting an error.
+   '0' means don't timeout.  */
+static void
+settimeout (double duration)
+{
+#if HAVE_SETITIMER
+  struct timeval tv;
+  struct timespec ts = dtotimespec (duration);
+  tv.tv_sec = MIN (ts.tv_sec, LONG_MAX); /* Assuming tv_sec is positive.  */
+  tv.tv_usec = (ts.tv_nsec + 999) / 1000;
+  if (tv.tv_usec == 1000 * 1000)
+    {
+      if (tv.tv_sec != LONG_MAX)
+        {
+          tv.tv_sec++;
+          tv.tv_usec = 0;
+        }
+      else
+        tv.tv_usec--;
+    }
+  struct itimerval it = { {0, 0}, tv };
+  if (setitimer (ITIMER_REAL, &it, NULL) == -1)
+    error (EXIT_FAILURE, errno, _("error setting timer"));
+#else
+  unsigned int timeint;
+  if (UINT_MAX <= duration)
+    timeint = UINT_MAX;
+  else
+    {
+      unsigned int duration_floor = duration;
+      timeint = duration_floor + (duration_floor < duration);
+    }
+  alarm (timeint);
+#endif
+}
+
 /* send sig to group but not ourselves.
  * FIXME: Is there a better way to achieve this?  */
 static int
@@ -125,7 +164,7 @@ cleanup (int sig)
         {
           /* Start a new timeout after which we'll send SIGKILL.  */
           term_signal = SIGKILL;
-          alarm (kill_after);
+          settimeout (kill_after);
           kill_after = 0; /* Don't let later signals reset kill alarm.  */
         }
 
@@ -182,7 +221,7 @@ Mandatory arguments to long options are mandatory for short options too.\n\
       fputs (VERSION_OPTION_DESCRIPTION, stdout);
 
       fputs (_("\n\
-DURATION is an integer with an optional suffix:\n\
+DURATION is a floating point number with an optional suffix:\n\
 `s' for seconds (the default), `m' for minutes, `h' for hours \
 or `d' for days.\n"), stdout);
 
@@ -232,7 +271,7 @@ apply_time_suffix (double *x, char suffix_char)
   return true;
 }
 
-static unsigned int
+static double
 parse_duration (const char* str)
 {
   double duration;
@@ -250,19 +289,7 @@ parse_duration (const char* str)
       usage (EXIT_CANCELED);
     }
 
-  /* Return the requested duration, rounded up to the next representable value.
-     Treat out-of-range values as if they were maximal,
-     as that's more useful in practice than reporting an error.
-
-     FIXME: Use dtotimespec + setitimer if setitimer is available,
-     as that has higher resolution.  */
-  if (UINT_MAX <= duration)
-    return UINT_MAX;
-  else
-    {
-      unsigned int duration_floor = duration;
-      return duration_floor + (duration_floor < duration);
-    }
+  return duration;
 }
 
 static void
@@ -285,7 +312,7 @@ install_signal_handlers (int sigterm)
 int
 main (int argc, char **argv)
 {
-  unsigned long timeout;
+  double timeout;
   char signame[SIG2STR_MAX];
   int c;
 
@@ -373,7 +400,7 @@ main (int argc, char **argv)
       pid_t wait_result;
       int status;
 
-      alarm (timeout);
+      settimeout (timeout);
 
       while ((wait_result = waitpid (monitored_pid, &status, 0)) < 0
              && errno == EINTR)
diff --git a/tests/misc/timeout-parameters b/tests/misc/timeout-parameters
index 56804a8..e99d4a5 100755
--- a/tests/misc/timeout-parameters
+++ b/tests/misc/timeout-parameters
@@ -55,6 +55,9 @@ test $? = 0 || fail=1
 timeout 2.34e+5d sleep 0
 test $? = 0 || fail=1
 
+# nanoseconds rounded up to microseconds
+timeout .999999999 sleep 0 || fail=1
+
 # invalid signal spec
 timeout --signal=invalid 1 sleep 0
 test $? = 125 || fail=1
-- 
1.7.5.2