Remove busy wait

Summary: Wait uses baton & callback running baton.post
when the original future is ready. However wrapping baton.post
with a funciton call (preparing a new value) adds the following race: baton.wait wakes up
before that function has call actually finished.
The explanation is the following: to prepare the value of the new future it's necessary
1. baton.post() 2. set the value (move constructor, memory operations, ...)
and this code is executed in a separate thread.
The main idea of this fix is to avoid creating a new future
(whose value is calculated using that 2-step procedure)
and set a callback instead. This callback will be executed when the future is ready and actually
it either will be the last step of promise.setValue or it will run immediately if the future
we are waiting for already contains a value.

Reviewed By: fugalh

Differential Revision: D2636409

fb-gh-sync-id: df3e9bbcc56a5fac5834ffecc63f1bcb94ace02c

diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h
index eeb0e2e56e759308d0862afab33ce0a42ee11b41..5979d183c35622bf079ea35762ae957cea1dc851 100644 (file)
--- a/folly/futures/Future-inl.h
+++ b/folly/futures/Future-inl.h
@@ -936,18 +936,8 @@ void waitImpl(Future<T>& f) {
   if (f.isReady()) return;
 
   folly::fibers::Baton baton;
-  f = f.then([&](Try<T> t) {
-    baton.post();
-    return makeFuture(std::move(t));
-  });
+  f.setCallback_([&](const Try<T>& t) { baton.post(); });
   baton.wait();
-
-  // 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.
-  while (!f.isReady()) {
-    std::this_thread::yield();
-  }
 }
 
 template <class T>
@@ -956,19 +946,10 @@ void waitImpl(Future<T>& f, Duration dur) {
   if (f.isReady()) return;
 
   auto baton = std::make_shared<folly::fibers::Baton>();
-  f = f.then([baton](Try<T> t) {
+  f.setCallback_([baton](const Try<T>& t) {
     baton->post();
-    return makeFuture(std::move(t));
   });
-
-  // Let's preserve the invariant that if we did not timeout (timed_wait returns
-  // true), then the returned Future is complete when it is returned to the
-  // caller. We need to wait out the race for that Future to complete.
-  if (baton->timed_wait(dur)) {
-    while (!f.isReady()) {
-      std::this_thread::yield();
-    }
-  }
+  baton->timed_wait(dur);
 }
 
 template <class T>