changeset 17388:3c592b4deb04

utimensat-tests, etc.: try to fix some races Problem reported by Bernhard Voelker in <http://lists.gnu.org/archive/html/bug-gnulib/2013-04/msg00071.html>. I don't know whether this patch fixes that race condition, but it fixes *some* race conditions, so it should be a win. * modules/chown-tests (Depends-on): * modules/fchownat-tests (Depends-on): * modules/fdutimensat-tests (Depends-on): * modules/futimens-tests (Depends-on): * modules/lchown-tests (Depends-on): * modules/stat-time-tests (Depends-on): * modules/utimens-tests (Depends-on): * modules/utimensat-tests (Depends-on): Depend on nanosleep, not usleep. * modules/chown-tests (test_chown_LDADD): * modules/lchown-tests (test_lchown_LDADD): * modules/stat-time-tests (test_stat_time_LDADD): New macro. * modules/fchownat-tests (test_fchownat_LDADD): * modules/fdutimensat-tests (test_fdutimensat_LDADD): * modules/futimens-tests (test_futimens_LDADD): * modules/utimens-tests (test_utimens_LDADD): * modules/utimensat-tests (test_utimensat_LDADD): Add $(LIB_NANOSLEEP). * modules/stat-time-tests (Files): Add tests/nap.h. * tests/nap.h: Include <limits.h>, for INT_MAX. (lt_mtime): Remove. (diff_timespec): New function. (get_stat): Rename from get_mtime. All callers changed. (nap_works): Determine the needed delay by inspecting the file system's timestamp jumps; this should be more reliable. Look at both mtime and ctime, and take the maximum of the two jumps. (nap_works, guess_delay): Return a nanosecond cound, not a microsecond count. All callers changed. (nap_works, nap): Use nanosleep, not usleep. Check for nanosleep failure. (nap): Multiply the guess by 1.125, to accommodate the case where the file system's clock is a bit slower than nanosleep's clock. * tests/test-stat-time.c (BASE): New macro. Include nap.h. (nap): Remove; nap.h now defines this. This removes a duplicate implementation of 'nap'.
author Paul Eggert <eggert@cs.ucla.edu>
date Tue, 30 Apr 2013 23:14:19 -0700
parents 5f320210ead1
children 1d9362a18c34
files ChangeLog modules/chown-tests modules/fchownat-tests modules/fdutimensat-tests modules/futimens-tests modules/lchown-tests modules/stat-time-tests modules/utimens-tests modules/utimensat-tests tests/nap.h tests/test-stat-time.c
diffstat 11 files changed, 139 insertions(+), 94 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,49 @@
 2013-04-30  Paul Eggert  <eggert@cs.ucla.edu>
 
+	utimensat-tests, etc.: try to fix some races
+	Problem reported by Bernhard Voelker in
+	<http://lists.gnu.org/archive/html/bug-gnulib/2013-04/msg00071.html>.
+	I don't know whether this patch fixes that race condition, but it
+	fixes *some* race conditions, so it should be a win.
+	* modules/chown-tests (Depends-on):
+	* modules/fchownat-tests (Depends-on):
+	* modules/fdutimensat-tests (Depends-on):
+	* modules/futimens-tests (Depends-on):
+	* modules/lchown-tests (Depends-on):
+	* modules/stat-time-tests (Depends-on):
+	* modules/utimens-tests (Depends-on):
+	* modules/utimensat-tests (Depends-on):
+	Depend on nanosleep, not usleep.
+	* modules/chown-tests (test_chown_LDADD):
+	* modules/lchown-tests (test_lchown_LDADD):
+	* modules/stat-time-tests (test_stat_time_LDADD):
+	New macro.
+	* modules/fchownat-tests (test_fchownat_LDADD):
+	* modules/fdutimensat-tests (test_fdutimensat_LDADD):
+	* modules/futimens-tests (test_futimens_LDADD):
+	* modules/utimens-tests (test_utimens_LDADD):
+	* modules/utimensat-tests (test_utimensat_LDADD):
+	Add $(LIB_NANOSLEEP).
+	* modules/stat-time-tests (Files): Add tests/nap.h.
+	* tests/nap.h: Include <limits.h>, for INT_MAX.
+	(lt_mtime): Remove.
+	(diff_timespec): New function.
+	(get_stat): Rename from get_mtime.  All callers changed.
+	(nap_works): Determine the needed delay by inspecting the
+	file system's timestamp jumps; this should be more reliable.
+	Look at both mtime and ctime, and take the maximum of the two jumps.
+	(nap_works, guess_delay):
+	Return a nanosecond cound, not a microsecond count.
+	All callers changed.
+	(nap_works, nap): Use nanosleep, not usleep.  Check for nanosleep
+	failure.
+	(nap): Multiply the guess by 1.125, to accommodate the case where
+	the file system's clock is a bit slower than nanosleep's clock.
+	* tests/test-stat-time.c (BASE): New macro.
+	Include nap.h.
+	(nap): Remove; nap.h now defines this.  This removes a duplicate
+	implementation of 'nap'.
+
 	utimens, utimensat: work around Solaris UTIME_OMIT bug
 	Solaris 11.1 and Solaris 10 have the same UTIME_OMIT bug that
 	Linux kernel 2.6.32 does.  Work around it in the same way.
--- a/modules/chown-tests
+++ b/modules/chown-tests
@@ -9,7 +9,7 @@
 ignore-value
 lstat
 mgetgroups
-usleep
+nanosleep
 stat-time
 stdbool
 symlink
@@ -20,3 +20,4 @@
 Makefile.am:
 TESTS += test-chown
 check_PROGRAMS += test-chown
+test_chown_LDADD = $(LDADD) $(LIB_NANOSLEEP)
--- a/modules/fchownat-tests
+++ b/modules/fchownat-tests
@@ -9,9 +9,9 @@
 Depends-on:
 ignore-value
 mgetgroups
+nanosleep
 openat-h
 progname
-usleep
 stat-time
 symlink
 
@@ -21,4 +21,4 @@
 Makefile.am:
 TESTS += test-fchownat
 check_PROGRAMS += test-fchownat
-test_fchownat_LDADD = $(LDADD) @LIBINTL@
+test_fchownat_LDADD = $(LDADD) $(LIB_NANOSLEEP) @LIBINTL@
--- a/modules/fdutimensat-tests
+++ b/modules/fdutimensat-tests
@@ -10,10 +10,10 @@
 Depends-on:
 fcntl-h
 ignore-value
+nanosleep
 openat
 timespec
 dup
-usleep
 utimecmp
 
 configure.ac:
@@ -21,4 +21,5 @@
 Makefile.am:
 TESTS += test-fdutimensat
 check_PROGRAMS += test-fdutimensat
-test_fdutimensat_LDADD = $(LDADD) $(LIB_CLOCK_GETTIME) @LIBINTL@
+test_fdutimensat_LDADD = $(LDADD) $(LIB_CLOCK_GETTIME) \
+  $(LIB_NANOSLEEP) @LIBINTL@
--- a/modules/futimens-tests
+++ b/modules/futimens-tests
@@ -10,9 +10,9 @@
 gettext
 fcntl-h
 ignore-value
+nanosleep
 timespec
 dup
-usleep
 utimecmp
 
 configure.ac:
@@ -20,4 +20,4 @@
 Makefile.am:
 TESTS += test-futimens
 check_PROGRAMS += test-futimens
-test_futimens_LDADD = $(LDADD) $(LIB_CLOCK_GETTIME) @LIBINTL@
+test_futimens_LDADD = $(LDADD) $(LIB_CLOCK_GETTIME) $(LIB_NANOSLEEP) @LIBINTL@
--- a/modules/lchown-tests
+++ b/modules/lchown-tests
@@ -8,7 +8,7 @@
 Depends-on:
 ignore-value
 mgetgroups
-usleep
+nanosleep
 stat-time
 stdbool
 symlink
@@ -19,3 +19,4 @@
 Makefile.am:
 TESTS += test-lchown
 check_PROGRAMS += test-lchown
+test_lchown_LDADD = $(LDADD) $(LIB_NANOSLEEP)
--- a/modules/stat-time-tests
+++ b/modules/stat-time-tests
@@ -1,13 +1,15 @@
 Files:
 tests/test-stat-time.c
 tests/macros.h
+tests/nap.h
 
 Depends-on:
+nanosleep
 time
-usleep
 
 configure.ac:
 
 Makefile.am:
 TESTS += test-stat-time
 check_PROGRAMS += test-stat-time
+test_stat_time_LDADD = $(LDADD) $(LIB_NANOSLEEP)
--- a/modules/utimens-tests
+++ b/modules/utimens-tests
@@ -11,9 +11,9 @@
 dup
 gettext
 ignore-value
+nanosleep
 symlink
 timespec
-usleep
 utimecmp
 
 configure.ac:
@@ -21,4 +21,4 @@
 Makefile.am:
 TESTS += test-utimens
 check_PROGRAMS += test-utimens
-test_utimens_LDADD = $(LDADD) $(LIB_CLOCK_GETTIME) @LIBINTL@
+test_utimens_LDADD = $(LDADD) $(LIB_CLOCK_GETTIME) $(LIB_NANOSLEEP) @LIBINTL@
--- a/modules/utimensat-tests
+++ b/modules/utimensat-tests
@@ -9,8 +9,8 @@
 
 Depends-on:
 ignore-value
+nanosleep
 timespec
-usleep
 utimecmp
 
 configure.ac:
@@ -18,4 +18,4 @@
 Makefile.am:
 TESTS += test-utimensat
 check_PROGRAMS += test-utimensat
-test_utimensat_LDADD = $(LDADD) $(LIB_CLOCK_GETTIME) @LIBINTL@
+test_utimensat_LDADD = $(LDADD) $(LIB_CLOCK_GETTIME) $(LIB_NANOSLEEP) @LIBINTL@
--- a/tests/nap.h
+++ b/tests/nap.h
@@ -19,19 +19,33 @@
 #ifndef GLTEST_NAP_H
 # define GLTEST_NAP_H
 
+# include <limits.h>
+
+/* Return A - B, in ns.
+   Return 0 if the true result would be negative.
+   Return INT_MAX if the true result would be greater than INT_MAX.  */
 static int
-lt_mtime (struct stat const *a, struct stat const *b)
+diff_timespec (struct timespec a, struct timespec b)
 {
-  time_t as = a->st_mtime;
-  time_t bs = b->st_mtime;
-  int ans = get_stat_mtime_ns (a);
-  int bns = get_stat_mtime_ns (b);
+  time_t as = a.tv_sec;
+  time_t bs = b.tv_sec;
+  int ans = a.tv_nsec;
+  int bns = b.tv_nsec;
 
-  return as < bs || (as == bs && ans < bns);
+  if (! (bs < as || (bs == as && bns < ans)))
+    return 0;
+  if (as - bs <= INT_MAX / 1000000000)
+    {
+      int sdiff = (as - bs) * 1000000000;
+      int usdiff = ans - bns;
+      if (usdiff < INT_MAX - sdiff)
+        return sdiff + usdiff;
+    }
+  return INT_MAX;
 }
 
 static void
-get_mtime (int fd, struct stat *st, int do_write)
+get_stat (int fd, struct stat *st, int do_write)
 {
   if (do_write)
     ASSERT (write (fd, "\n", 1) == 1);
@@ -39,50 +53,61 @@
 }
 
 /* Given a file whose descriptor is FD, see whether delaying by DELAY
-   microseconds causes a change in a file's time stamp.  If the time
-   stamps differ, repeat the test one more time, in case we crossed a
-   quantization boundary on a file system with lower resolution.  *ST
-   is the file's status, recently gotten.  Update *ST to reflect the
-   latest status gotten.  */
+   nanoseconds causes a change in a file's time stamp.  *ST is the
+   file's status, recently gotten.  Update *ST to reflect the latest
+   status gotten.  If successful, return the needed delay, in
+   nanoseconds as determined by the observed time stamps; this may be
+   greater than DELAY if we crossed a quantization boundary.  If
+   unsuccessful, return 0.  */
 static int
 nap_works (int fd, int delay, struct stat *st)
 {
-  struct stat old_st;
-  old_st = *st;
-  usleep (delay);
-  get_mtime (fd, st, 1);
-  if (! lt_mtime (&old_st, st))
-    return 0;
-  old_st = *st;
-  usleep (delay);
-  get_mtime (fd, st, 1);
-  return lt_mtime (&old_st, st);
+  struct stat old_st = *st;
+  struct timespec delay_spec;
+  int cdiff, mdiff;
+  delay_spec.tv_sec = delay / 1000000000;
+  delay_spec.tv_nsec = delay % 1000000000;
+  ASSERT (nanosleep (&delay_spec, 0) == 0);
+  get_stat (fd, st, 1);
+
+  /* Return the greater of the ctime and the mtime differences, or
+     zero if it cannot be determined, or INT_MAX if either overflows.  */
+  cdiff = diff_timespec (get_stat_ctime (st), get_stat_ctime (&old_st));
+  if (cdiff != 0)
+    {
+      mdiff = diff_timespec (get_stat_mtime (st), get_stat_mtime (&old_st));
+      if (mdiff != 0)
+        return cdiff < mdiff ? mdiff : cdiff;
+    }
+  return 0;
 }
 
 static int
 guess_delay (void)
 {
-  /* Try a 1-microsecond sleep first, for speed.  If that doesn't
-     work, try a 1 ms sleep; that should work with ext.  If it doesn't
-     work, try a 20 ms sleep.  xfs has a quantization of about 10
+  /* Try a 1-ns sleep first, for speed.  If that doesn't work, try 100
+     ns, 1 microsecond, 1 ms, etc.  xfs has a quantization of about 10
      milliseconds, even though it has a granularity of 1 nanosecond,
      and NTFS has a default quantization of 15.25 milliseconds, even
-     though it has a granularity of 100 nanoseconds, so 20 ms is a
+     though it has a granularity of 100 nanoseconds, so 15.25 ms is a
      good quantization to try.  If that doesn't work, try 1 second.
      The worst case is 2 seconds, needed for FAT.  */
-  static int const delaytab[] = {1, 1000, 20000, 1000000 };
+  static int const delaytab[] = {1, 1000, 1000000, 15250000, 1000000000 };
   int fd = creat (BASE "tmp", 0600);
   int i;
-  int delay = 2000000;
+  int delay = 2000000000;
   struct stat st;
   ASSERT (0 <= fd);
-  get_mtime (fd, &st, 0);
+  get_stat (fd, &st, 0);
   for (i = 0; i < sizeof delaytab / sizeof delaytab[0]; i++)
-    if (nap_works (fd, delaytab[i], &st))
-      {
-        delay = delaytab[i];
-        break;
-      }
+    {
+      int d = nap_works (fd, delaytab[i], &st);
+      if (d != 0)
+        {
+          delay = d;
+          break;
+        }
+    }
   ASSERT (close (fd) == 0);
   ASSERT (unlink (BASE "tmp") == 0);
   return delay;
@@ -90,14 +115,24 @@
 
 /* Sleep long enough to notice a timestamp difference on the file
    system in the current directory.  Assumes that BASE is defined,
-   and requires that the test module depends on usleep.  */
+   and requires that the test module depends on nanosleep.  */
 static void
 nap (void)
 {
-  static int delay;
-  if (!delay)
-    delay = guess_delay ();
-  usleep (delay);
+  static struct timespec delay;
+  if (!delay.tv_sec && !delay.tv_nsec)
+    {
+      int d = guess_delay ();
+
+      /* Multiply by 1.125 (rounding up), to avoid problems if the
+         file system's clock is a bit slower than nanosleep's.
+         Ceiling it at INT_MAX, though.  */
+      int delta = (d >> 3) + ((d & 7) != 0);
+      d = delta < INT_MAX - d ? d + delta : INT_MAX;
+      delay.tv_sec = d / 1000000000;
+      delay.tv_nsec = d % 1000000000;
+    }
+  ASSERT (nanosleep (&delay, 0) == 0);
 }
 
 #endif /* GLTEST_NAP_H */
--- a/tests/test-stat-time.c
+++ b/tests/test-stat-time.c
@@ -27,6 +27,9 @@
 
 #include "macros.h"
 
+#define BASE "test-stat-time.t"
+#include "nap.h"
+
 enum { NFILES = 4 };
 
 static int
@@ -79,48 +82,6 @@
   ASSERT (stat (filename, p) == 0);
 }
 
-/* Sleep long enough to notice a timestamp difference on the file
-   system in the current directory.  */
-static void
-nap (void)
-{
-  static long delay;
-  if (!delay)
-    {
-      /* Initialize only once, by sleeping for 20 milliseconds (needed
-         since xfs has a quantization of about 10 milliseconds, even
-         though it has a granularity of 1 nanosecond, and since NTFS
-         has a default quantization of 15.25 milliseconds, even though
-         it has a granularity of 100 nanoseconds).  If the seconds
-         differ, repeat the test one more time (in case we crossed a
-         quantization boundary on a file system with 1 second
-         resolution).  If we can't observe a difference in only the
-         nanoseconds, then fall back to 1 second if the time is odd,
-         and 2 seconds (needed for FAT) if time is even.  */
-      struct stat st1;
-      struct stat st2;
-      ASSERT (stat ("t-stt-stamp1", &st1) == 0);
-      ASSERT (force_unlink ("t-stt-stamp1") == 0);
-      delay = 20000;
-      usleep (delay);
-      create_file ("t-stt-stamp1");
-      ASSERT (stat ("t-stt-stamp1", &st2) == 0);
-      if (st1.st_mtime != st2.st_mtime)
-        {
-          /* Seconds differ, give it one more shot.  */
-          st1 = st2;
-          ASSERT (force_unlink ("t-stt-stamp1") == 0);
-          usleep (delay);
-          create_file ("t-stt-stamp1");
-          ASSERT (stat ("t-stt-stamp1", &st2) == 0);
-        }
-      if (! (st1.st_mtime == st2.st_mtime
-             && get_stat_mtime_ns (&st1) < get_stat_mtime_ns (&st2)))
-        delay = (st1.st_mtime & 1) ? 1000000 : 2000000;
-    }
-  usleep (delay);
-}
-
 static void
 prepare_test (struct stat *statinfo, struct timespec *modtimes)
 {