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 6364bf0c5279efc1fa12580c90131e8ca7597971..d0392bbf94cd443ea1496236283bdc3dd859d399 100644 (file)
--- 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 c068df065ed9fbc28f749ba6ac8a6df3bb3046ab..0d1710066efdfb0cebd582f33e5b2e63be883fae 100644 (file)
--- 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 b2c4883b0d9436585d7902e9ea356ec76e30ee6b..ae26628d8ee52bc40401b08144a7a1b250bd3a8c 100644 (file)
--- 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 f3d6f00a82053983b0852daba9cdf7d31ef8fbcd..0c7b77f0b3497a3b553d481ca0d9ae1d9c82b638 100644 (file)
--- 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();
+}