Revert "[futures] waitWithSemaphore -> Future<T>::wait()"

Summary:
Subject:

This reverts commit d8eeb9a12f932b74d268c74ce7ce2610dc14b152.

Test Plan: Watch contbuild?

Reviewed By: njormrod@fb.com

Subscribers: trunkagent, doug, fbcode-common-diffs@, hero-diffs@, cold-storage-diffs@, adamsyta, zhuohuang, darshan, micha, folly-diffs@, lins, tingy, hannesr, jsedgwick

FB internal diff: D1797163

Tasks: 5940008

Signature: t1:1797163:1421959040:c9d528367fed2de2f7dafdac9416aa01bdb47df9

diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h
index 34940cedb39d57c410c5182356a86128dc38430a..4b12706f0661703d55704128f46d14affbcaa269 100644 (file)
--- a/folly/futures/Future-inl.h
+++ b/folly/futures/Future-inl.h
@@ -643,6 +643,65 @@ whenN(InputIterator first, InputIterator last, size_t n) {
   return ctx->p.getFuture();
 }
 
+template <typename T>
+Future<T>
+waitWithSemaphore(Future<T>&& f) {
+  Baton<> baton;
+  auto done = f.then([&](Try<T> &&t) {
+    baton.post();
+    return std::move(t.value());
+  });
+  baton.wait();
+  while (!done.isReady()) {
+    // There's a race here between the return here and the actual finishing of
+    // the future. f is completed, but the setup may not have finished on done
+    // after the baton has posted.
+    std::this_thread::yield();
+  }
+  return done;
+}
+
+template<>
+inline Future<void> waitWithSemaphore<void>(Future<void>&& f) {
+  Baton<> baton;
+  auto done = f.then([&](Try<void> &&t) {
+    baton.post();
+    t.value();
+  });
+  baton.wait();
+  while (!done.isReady()) {
+    // There's a race here between the return here and the actual finishing of
+    // the future. f is completed, but the setup may not have finished on done
+    // after the baton has posted.
+    std::this_thread::yield();
+  }
+  return done;
+}
+
+template <typename T, class Dur>
+Future<T>
+waitWithSemaphore(Future<T>&& f, Dur timeout) {
+  auto baton = std::make_shared<Baton<>>();
+  auto done = f.then([baton](Try<T> &&t) {
+    baton->post();
+    return std::move(t.value());
+  });
+  baton->timed_wait(std::chrono::system_clock::now() + timeout);
+  return done;
+}
+
+template <class Dur>
+Future<void>
+waitWithSemaphore(Future<void>&& f, Dur timeout) {
+  auto baton = std::make_shared<Baton<>>();
+  auto done = f.then([baton](Try<void> &&t) {
+    baton->post();
+    t.value();
+  });
+  baton->timed_wait(std::chrono::system_clock::now() + timeout);
+  return done;
+}
+
 namespace {
   template <class T>
   void getWaitHelper(Future<T>* f) {

diff --git a/folly/futures/Future.h b/folly/futures/Future.h
index 7d210a76b7936778688a704cdfde7868e59e6f14..948e263a22b3bc6e4dca80afc5289bbed37d9077 100644 (file)
--- a/folly/futures/Future.h
+++ b/folly/futures/Future.h
@@ -631,6 +631,24 @@ Future<std::vector<std::pair<
   Try<typename std::iterator_traits<InputIterator>::value_type::value_type>>>>
 whenN(InputIterator first, InputIterator last, size_t n);
 
+/** Wait for the given future to complete on a semaphore. Returns a completed
+ * future containing the result.
+ *
+ * NB if the promise for the future would be fulfilled in the same thread that
+ * you call this, it will deadlock.
+ */
+template <class T>
+Future<T> waitWithSemaphore(Future<T>&& f);
+
+/** Wait for up to `timeout` for the given future to complete. Returns a future
+ * which may or may not be completed depending whether the given future
+ * completed in time
+ *
+ * Note: each call to this starts a (short-lived) thread and allocates memory.
+ */
+template <typename T, class Dur>
+Future<T> waitWithSemaphore(Future<T>&& f, Dur timeout);
+
 } // folly
 
 #include <folly/futures/Future-inl.h>

diff --git a/folly/futures/test/FutureTest.cpp b/folly/futures/test/FutureTest.cpp
index dad82fd37ed325f2f8d0c57dcd49e20c3b18fdc4..a69c8b9671c4975ccbbf98a5d71c8c882160eb9c 100644 (file)
--- a/folly/futures/test/FutureTest.cpp
+++ b/folly/futures/test/FutureTest.cpp
@@ -874,31 +874,31 @@ TEST(Future, throwIfFailed) {
     });
 }
 
-TEST(Future, waitImmediate) {
-  makeFuture().wait();
-  auto done = makeFuture(42).wait().value();
+TEST(Future, waitWithSemaphoreImmediate) {
+  waitWithSemaphore(makeFuture());
+  auto done = waitWithSemaphore(makeFuture(42)).value();
   EXPECT_EQ(42, done);
 
   vector<int> v{1,2,3};
-  auto done_v = makeFuture(v).wait().value();
+  auto done_v = waitWithSemaphore(makeFuture(v)).value();
   EXPECT_EQ(v.size(), done_v.size());
   EXPECT_EQ(v, done_v);
 
   vector<Future<void>> v_f;
   v_f.push_back(makeFuture());
   v_f.push_back(makeFuture());
-  auto done_v_f = whenAll(v_f.begin(), v_f.end()).wait().value();
+  auto done_v_f = waitWithSemaphore(whenAll(v_f.begin(), v_f.end())).value();
   EXPECT_EQ(2, done_v_f.size());
 
   vector<Future<bool>> v_fb;
   v_fb.push_back(makeFuture(true));
   v_fb.push_back(makeFuture(false));
   auto fut = whenAll(v_fb.begin(), v_fb.end());
-  auto done_v_fb = std::move(fut.wait().value());
+  auto done_v_fb = std::move(waitWithSemaphore(std::move(fut)).value());
   EXPECT_EQ(2, done_v_fb.size());
 }
 
-TEST(Future, wait) {
+TEST(Future, waitWithSemaphore) {
   Promise<int> p;
   Future<int> f = p.getFuture();
   std::atomic<bool> flag{false};
@@ -911,7 +911,7 @@ TEST(Future, wait) {
           return t.value();
         });
       flag = true;
-      result.store(n.wait().value());
+      result.store(waitWithSemaphore(std::move(n)).value());
     },
     std::move(f)
     );
@@ -925,11 +925,12 @@ TEST(Future, wait) {
   EXPECT_EQ(result.load(), 42);
 }
 
-TEST(Future, waitWithDuration) {
+TEST(Future, waitWithSemaphoreForTime) {
  {
   Promise<int> p;
   Future<int> f = p.getFuture();
-  auto t = f.wait(std::chrono::milliseconds(1));
+  auto t = waitWithSemaphore(std::move(f),
+    std::chrono::microseconds(1));
   EXPECT_FALSE(t.isReady());
   p.setValue(1);
   EXPECT_TRUE(t.isReady());
@@ -938,7 +939,8 @@ TEST(Future, waitWithDuration) {
   Promise<int> p;
   Future<int> f = p.getFuture();
   p.setValue(1);
-  auto t = f.wait(std::chrono::milliseconds(1));
+  auto t = waitWithSemaphore(std::move(f),
+    std::chrono::milliseconds(1));
   EXPECT_TRUE(t.isReady());
  }
  {
@@ -946,7 +948,8 @@ TEST(Future, waitWithDuration) {
   v_fb.push_back(makeFuture(true));
   v_fb.push_back(makeFuture(false));
   auto f = whenAll(v_fb.begin(), v_fb.end());
-  auto t = f.wait(std::chrono::milliseconds(1));
+  auto t = waitWithSemaphore(std::move(f),
+    std::chrono::milliseconds(1));
   EXPECT_TRUE(t.isReady());
   EXPECT_EQ(2, t.value().size());
  }
@@ -957,7 +960,8 @@ TEST(Future, waitWithDuration) {
   v_fb.push_back(p1.getFuture());
   v_fb.push_back(p2.getFuture());
   auto f = whenAll(v_fb.begin(), v_fb.end());
-  auto t = f.wait(std::chrono::milliseconds(1));
+  auto t = waitWithSemaphore(std::move(f),
+    std::chrono::milliseconds(1));
   EXPECT_FALSE(t.isReady());
   p1.setValue(true);
   EXPECT_FALSE(t.isReady());
@@ -965,7 +969,8 @@ TEST(Future, waitWithDuration) {
   EXPECT_TRUE(t.isReady());
  }
  {
-  auto t = makeFuture().wait(std::chrono::milliseconds(1));
+  auto t = waitWithSemaphore(makeFuture(),
+    std::chrono::milliseconds(1));
   EXPECT_TRUE(t.isReady());
  }
 }
@@ -1220,7 +1225,7 @@ TEST(Future, t5506504) {
     return whenAll(futures.begin(), futures.end());
   };
 
-  fn().wait();
+  waitWithSemaphore(fn());
 }
 
 // Test of handling of a circular dependency. It's never recommended