From 52b003afab5e0bae90dbc773893ef604dd3d68c1 Mon Sep 17 00:00:00 2001
From: Alexander Shaposhnikov <alexshap@fb.com>
Date: Tue, 10 Nov 2015 15:42:18 -0800
Subject: [PATCH] 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
---
 folly/futures/Future-inl.h | 25 +++----------------------
 1 file changed, 3 insertions(+), 22 deletions(-)

diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h
index eeb0e2e5..5979d183 100644
--- 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>
-- 
2.34.1