fix Futex when steady_clock and system_clock precisions differ

Summary:
To handle the strange relationship between steady_clock and
system_clock (on some platforms these represent only one type, on some
platforms they are separate) Futex::futexWaitUntil converts the deadline
to a duration and back.  On Xcode 6 system_clock is measured in
microseconds and steady_clock in nanoseconds, resulting in a compilation
failure.  This diff fixes it.

Test Plan:
1. compile snippet manually using Xcode
2. new unit test that causes the same implicit conversion failure in a slightly different way

Reviewed By: mssarang@fb.com

Subscribers: trunkagent, folly-diffs@

FB internal diff: D1740903

Tasks: 5831196

Signature: t1:1740903:1418754770:32c999abf1dc87415ffebf45083a903abbded9f2

diff --git a/folly/detail/Futex.h b/folly/detail/Futex.h
index fe61d0e66417081fcc45c5a569b3b492acbf0306..a38b4423e057f1ee94e0cddb792417d807832fb6 100644 (file)
--- a/folly/detail/Futex.h
+++ b/folly/detail/Futex.h
@@ -80,12 +80,24 @@ struct Futex : Atom<uint32_t>, boost::noncopyable {
         "futexWaitUntil only knows std::chrono::{system_clock,steady_clock}");
     assert((std::is_same<Clock, system_clock>::value) || Clock::is_steady);
 
-    auto duration = absTime.time_since_epoch();
+    // We launder the clock type via a std::chrono::duration so that we
+    // can compile both the true and false branch.  Tricky case is when
+    // steady_clock has a higher precision than system_clock (Xcode 6,
+    // for example), for which time_point<system_clock> construction
+    // refuses to do an implicit duration conversion.  (duration is
+    // happy to implicitly convert its denominator causing overflow, but
+    // refuses conversion that might cause truncation.)  We use explicit
+    // duration_cast to work around this.  Truncation does not actually
+    // occur (unless Duration != Clock::duration) because the missing
+    // implicit conversion is in the untaken branch.
+    Duration absTimeDuration = absTime.time_since_epoch();
     if (std::is_same<Clock, system_clock>::value) {
-      time_point<system_clock> absSystemTime(duration);
+      time_point<system_clock> absSystemTime(
+          duration_cast<system_clock::duration>(absTimeDuration));
       return futexWaitImpl(expected, &absSystemTime, nullptr, waitMask);
     } else {
-      time_point<steady_clock> absSteadyTime(duration);
+      time_point<steady_clock> absSteadyTime(
+          duration_cast<steady_clock::duration>(absTimeDuration));
       return futexWaitImpl(expected, nullptr, &absSteadyTime, waitMask);
     }
   }

diff --git a/folly/test/FutexTest.cpp b/folly/test/FutexTest.cpp
index 3c5c63d754ee312c87f9f76cf4ceb4316927b363..71392bfef1396a56c56ac7a8e4856674d5c5e984 100644 (file)
--- a/folly/test/FutexTest.cpp
+++ b/folly/test/FutexTest.cpp
@@ -19,6 +19,7 @@
 
 #include <chrono>
 #include <functional>
+#include <ratio>
 #include <thread>
 
 #include <gflags/gflags.h>
@@ -28,6 +29,7 @@
 
 using namespace folly::detail;
 using namespace folly::test;
+using namespace std;
 using namespace std::chrono;
 
 typedef DeterministicSchedule DSched;
@@ -54,7 +56,7 @@ void run_basic_tests() {
   DSched::join(thr);
 }
 
-template <template<typename> class Atom, typename Clock>
+template <template<typename> class Atom, typename Clock, typename Duration>
 void liveClockWaitUntilTests() {
   Futex<Atom> f(0);
 
@@ -62,7 +64,8 @@ void liveClockWaitUntilTests() {
     auto fp = &f; // workaround for t5336595
     auto thrA = DSched::thread([fp,stress]{
       while (true) {
-        auto deadline = Clock::now() + microseconds(1 << (stress % 20));
+        auto deadline = time_point_cast<Duration>(
+            Clock::now() + microseconds(1 << (stress % 20)));
         auto res = fp->futexWaitUntil(0, deadline);
         EXPECT_TRUE(res == FutexResult::TIMEDOUT || res == FutexResult::AWOKEN);
         if (res == FutexResult::AWOKEN) {
@@ -98,8 +101,11 @@ void deterministicAtomicWaitUntilTests() {
 
 template<template<typename> class Atom>
 void run_wait_until_tests() {
-  liveClockWaitUntilTests<Atom, system_clock>();
-  liveClockWaitUntilTests<Atom, steady_clock>();
+  liveClockWaitUntilTests<Atom, system_clock, system_clock::duration>();
+  liveClockWaitUntilTests<Atom, steady_clock, steady_clock::duration>();
+
+  typedef duration<int64_t, pico> picoseconds;
+  liveClockWaitUntilTests<Atom, system_clock, picoseconds>();
 }
 
 template <>