Modification to futures to remove deadlock in certain use cases for getVia(executor).

Summary: If getVia was called on a future modified using via, getVia could deadlock if the original future was updated to a new executor and there was no callback chained after the call to via. In effect: f.via(executor).getVia(executor); deadlocks. This can be a problem if the code is hidden in a library and the precise semantics are unclear. This diff adds a test that exposes the problem and a fix by forcing waitVia to add a callback that will satisfy the new exector, ensuring that drive() has a callback to trigger once the future is satisfied.

Reviewed By: andriigrynenko

Differential Revision: D2906858

fb-gh-sync-id: a3105079530f15d7a7d39a9381c4078665b721a7
shipit-source-id: a3105079530f15d7a7d39a9381c4078665b721a7

folly/futures/Future-inl.h

@@ -983,9 +983,16 @@ void waitImpl(Future<T>& f, Duration dur) {
 
 template <class T>
 void waitViaImpl(Future<T>& f, DrivableExecutor* e) {
+  // Set callback so to ensure that the via executor has something on it
+  // so that once the preceding future triggers this callback, drive will
+  // always have a callback to satisfy it
+  if (f.isReady())
+    return;
+  f = f.then([](T&& t) { return std::move(t); });
   while (!f.isReady()) {
     e->drive();
   }
+  assert(f.isReady());
 }
 
 } // detail

folly/futures/test/ExecutorTest.cpp

@@ -106,6 +106,21 @@ TEST(ManualExecutor, waitForDoesNotDeadlock) {
   t.join();
 }
 
+TEST(ManualExecutor, getViaDoesNotDeadlock) {
+  ManualExecutor east, west;
+  folly::Baton<> baton;
+  auto f = makeFuture().via(&east).then([](Try<Unit>) {
+    return makeFuture();
+  }).via(&west);
+  std::thread t([&] {
+    baton.post();
+    f.getVia(&west);
+  });
+  baton.wait();
+  east.run();
+  t.join();
+}
+
 TEST(Executor, InlineExecutor) {
   InlineExecutor x;
   size_t counter = 0;