From: Dave Watson <davejwatson@fb.com>
Date: Tue, 2 Aug 2016 21:05:35 +0000 (-0700)
Subject: Ensure getVia(eventbase) does not busy wait
X-Git-Tag: v2016.08.08.00~37
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=62cc06728355f8f1cdaff8b466756fa762a277b1;p=folly.git

Ensure getVia(eventbase) does not busy wait

Summary:
Currently, getVia(eventbase) will busy wait if no work is scheduled on the event base.

Tweak the DrivableExecutor API a bit to support sleeping/wakeups.
There was already a similar fix for the only other existing DrivableExecutor, the ManualExecutor, in
D2906858.

Reviewed By: andriigrynenko

Differential Revision: D3613954

fbshipit-source-id: 9ff9f2e010040d9886fdf51a665e3afabbff57c0
---

diff --git a/folly/futures/DrivableExecutor.h b/folly/futures/DrivableExecutor.h
index 6364bf0c..d0392bbf 100644
--- a/folly/futures/DrivableExecutor.h
+++ b/folly/futures/DrivableExecutor.h
@@ -36,11 +36,19 @@ namespace folly {
  * These will be most helpful in tests, for instance if you need to pump a mock
  * EventBase until Futures complete.
  */
+
 class DrivableExecutor : public virtual Executor {
  public:
   virtual ~DrivableExecutor() = default;
 
   // Make progress on this Executor's work.
+  //
+  // Drive *must not* busy wait if there is no work to do.  Instead,
+  // sleep (using a semaphore or similar) until at least one event is
+  // processed.
+  // I.e. make_future().via(foo).then(...).getVia(DrivableExecutor)
+  // must not spin, even though nothing happens on the drivable
+  // executor.
   virtual void drive() = 0;
 };
 
diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h
index c068df06..0d171006 100644
--- a/folly/futures/Future-inl.h
+++ b/folly/futures/Future-inl.h
@@ -987,7 +987,7 @@ void waitViaImpl(Future<T>& f, DrivableExecutor* e) {
   // always have a callback to satisfy it
   if (f.isReady())
     return;
-  f = f.then([](T&& t) { return std::move(t); });
+  f = f.via(e).then([](T&& t) { return std::move(t); });
   while (!f.isReady()) {
     e->drive();
   }
diff --git a/folly/io/async/EventBase.h b/folly/io/async/EventBase.h
index b2c4883b..ae26628d 100644
--- a/folly/io/async/EventBase.h
+++ b/folly/io/async/EventBase.h
@@ -592,6 +592,7 @@ class EventBase : private boost::noncopyable,
 
   /// Implements the DrivableExecutor interface
   void drive() override {
+    auto keepAlive = loopKeepAlive();
     loopOnce();
   }
 
diff --git a/folly/io/async/test/EventBaseTest.cpp b/folly/io/async/test/EventBaseTest.cpp
index f3d6f00a..0c7b77f0 100644
--- a/folly/io/async/test/EventBaseTest.cpp
+++ b/folly/io/async/test/EventBaseTest.cpp
@@ -27,6 +27,8 @@
 #include <folly/io/async/test/Util.h>
 #include <folly/portability/Unistd.h>
 
+#include <folly/futures/Promise.h>
+
 #include <atomic>
 #include <iostream>
 #include <memory>
@@ -1822,3 +1824,32 @@ TEST(EventBaseTest, LoopKeepAliveShutdown) {
 
   t.join();
 }
+
+TEST(EventBaseTest, DrivableExecutorTest) {
+  folly::Promise<bool> p;
+  auto f = p.getFuture();
+  EventBase base;
+  bool finished = false;
+
+  std::thread t([&] {
+    /* sleep override */
+    std::this_thread::sleep_for(std::chrono::microseconds(10));
+    finished = true;
+    base.runInEventBaseThread([&]() { p.setValue(true); });
+  });
+
+  // Ensure drive does not busy wait
+  base.drive(); // TODO: fix notification queue init() extra wakeup
+  base.drive();
+  EXPECT_TRUE(finished);
+
+  folly::Promise<bool> p2;
+  auto f2 = p2.getFuture();
+  // Ensure waitVia gets woken up properly, even from
+  // a separate thread.
+  base.runAfterDelay([&]() { p2.setValue(true); }, 10);
+  f2.waitVia(&base);
+  EXPECT_TRUE(f2.isReady());
+
+  t.join();
+}