Re: [bug-gnulib] detecting nonconforming nanosleep

[Paul Eggert]

Jim Meyering <address@hidden> writes:

>       static const struct timespec child_sleep = { 1, 0 };
> ...
>       static const struct timespec delay = { 0, 300000000 };

A minor portability nit: POSIX doesn't let you assume that tv_sec
precedes tv_nsec, or that there are only two members in struct
timespec.  So you have to rewrite these by using explicit assignment
to the members of child_sleep and delay.

>         || kill (pid, SIGTSTP) != 0
>         || kill (pid, SIGCONT) != 0

Don't you need to sleep a little between sending the two signals?
<http://www.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html#tag_02_04_01>
says "when SIGCONT is generated for a process, all pending stop
signals for that process shall be discarded."  (On the other hand, if
you know that the buggy versions of the Linux kernel can't possibly
cause the TSTP to be pending, then there's no point waiting.)

There is the larger problem, that we are using a configure-time test,
so a program built on (say) Linux 2.4.7 won't run under 2.4.8.


Come to think of it, how about the following idea instead?
Let's remove the workaround entirely.

This will cause "sleep" to sleep for too long on some buggy Linux
kernels, but the behavior will still conform to POSIX, as POSIX allows
"sleep" etc. to sleep for a longer time than requested.  This will
greatly simplify our configuration hassles, and will cause the "sleep"
command to no longer need -lrt.  (And if someone complains, we can ask
them to fix their kernel.  :-)

Here's a proposed coreutils patch along these lines.

2005-02-28  Paul Eggert  <address@hidden>

        Remove workaround for bug in Linux kernel 2.6.8 or thereabouts.
        The workaround isn't strictly needed for POSIX conformance, and
        it's too much of a pain to configure and maintain.  We'll ask
        people to fix their kernels instead.
        * lib/xnanosleep: Don't include gethrxtime.h or xtime.h.
        (NANOSLEEP_BUG_WORKAROUND): Remove.
        (xnanosleep): Remove the workaround.
        * m4/xnanosleep.m4 (gl_XNANOSLEEP): Remove configuration attempting
        to detect nanosleep bug.

Index: lib/xnanosleep.c
===================================================================
RCS file: /fetish/cu/lib/xnanosleep.c,v
retrieving revision 1.12
diff -p -u -r1.12 xnanosleep.c
--- lib/xnanosleep.c    28 Feb 2005 10:02:58 -0000      1.12
+++ lib/xnanosleep.c    28 Feb 2005 19:15:49 -0000
@@ -33,8 +33,6 @@
 #include <time.h>
 
 #include "timespec.h"
-#include "gethrxtime.h"
-#include "xtime.h"
 
 /* The extra casts work around common compiler bugs.  */
 #define TYPE_SIGNED(t) (! ((t) 0 < (t) -1))
@@ -48,19 +46,6 @@
 # define TIME_T_MAX TYPE_MAXIMUM (time_t)
 #endif
 
-/* POSIX.1-2001 requires that when a process is suspended, then
-   resumed, nanosleep (A, B) returns -1, sets errno to EINTR, and sets
-   *B to the time remaining at the point of resumption.  However, some
-   versions of the Linux kernel incorrectly return the time remaining
-   at the point of suspension.  Work around this bug on GNU/Linux
-   hosts by computing the remaining time here after nanosleep returns,
-   rather than by relying on nanosleep's computation.  */
-#ifdef __linux__
-enum { NANOSLEEP_BUG_WORKAROUND = true };
-#else
-enum { NANOSLEEP_BUG_WORKAROUND = false };
-#endif
-
 /* Sleep until the time (call it WAKE_UP_TIME) specified as
    SECONDS seconds after the time this function is called.
    SECONDS must be non-negative.  If SECONDS is so large that
@@ -76,19 +61,9 @@ xnanosleep (double seconds)
   bool overflow = false;
   double ns;
   struct timespec ts_sleep;
-  xtime_t stop = 0;
 
   assert (0 <= seconds);
 
-  if (NANOSLEEP_BUG_WORKAROUND)
-    {
-      xtime_t now = gethrxtime ();
-      double increment = XTIME_PRECISION * seconds;
-      xtime_t incr = increment;
-      stop = now + incr + (incr < increment);
-      overflow = (stop < now);
-    }
-
   /* Separate whole seconds from nanoseconds.
      Be careful to detect any overflow.  */
   ts_sleep.tv_sec = seconds;
@@ -134,19 +109,6 @@ xnanosleep (double seconds)
        break;
       if (errno != EINTR && errno != 0)
        return -1;
-
-      if (NANOSLEEP_BUG_WORKAROUND)
-       {
-         xtime_t now = gethrxtime ();
-         if (stop <= now)
-           break;
-         else
-           {
-             xtime_t remaining = stop - now;
-             ts_sleep.tv_sec = xtime_nonnegative_sec (remaining);
-             ts_sleep.tv_nsec = xtime_nonnegative_nsec (remaining);
-           }
-       }
     }
 
   return 0;
Index: m4/xnanosleep.m4
===================================================================
RCS file: /fetish/cu/m4/xnanosleep.m4,v
retrieving revision 1.1
diff -p -u -r1.1 xnanosleep.m4
--- m4/xnanosleep.m4    22 Feb 2005 07:06:47 -0000      1.1
+++ m4/xnanosleep.m4    28 Feb 2005 19:15:49 -0000
@@ -1,4 +1,4 @@
-# xnanosleep.m4 serial 1
+# xnanosleep.m4 serial 2
 dnl Copyright (C) 2005 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -10,26 +10,4 @@ AC_DEFUN([gl_XNANOSLEEP],
 [
   AC_LIBSOURCES([xnanosleep.c, xnanosleep.h])
   AC_LIBOBJ([xnanosleep])
-
-  dnl Prerequisites of lib/xnanosleep.c.
-  AC_REQUIRE([gl_PREREQ_GETHRXTIME])
-
-  LIB_XNANOSLEEP=
-  case $LIB_GETHRXTIME in
-  ?*)
-    AC_CACHE_CHECK([whether __linux__ is defined],
-      gl_cv_have___linux__,
-      [AC_EGREP_CPP([have___linux__],
-       [
-#        ifdef __linux__
-         have___linux__
-#        endif
-       ],
-       gl_cv_have___linux__=yes,
-       gl_cv_have___linux__=no)])
-    if test $gl_cv_have___linux__ = yes; then
-      LIB_XNANOSLEEP=$LIB_GETHRXTIME
-    fi;;
-  esac
-  AC_SUBST([LIB_XNANOSLEEP])
 ])