Make outstanding LoopKeepAlive hold EventBase destructor

Summary:
LoopKeepAlive handle can be grabbed to signal that some external event will later be scheduled on the EventBase via runInEventBaseThread. Usually the code which will be calling runInEventBaseThread only has a raw pointer to an EventBase, so it doesn't have any way to know it was destroyed.

This change ensures that EventBase destructor will keep running the event loop until all such LoopKeepAlive handles are released.

Reviewed By: yfeldblum

Differential Revision: D3323835

fbshipit-source-id: 4071dae691a61dfebe2f1759cf99f661b161fa4a

diff --git a/folly/io/async/EventBase.cpp b/folly/io/async/EventBase.cpp
index 5c0b78129e37a39e9f4da5ecbba45604b9363710..0987f1fac282cce125b41c7bea482e58666b9f37 100644 (file)
--- a/folly/io/async/EventBase.cpp
+++ b/folly/io/async/EventBase.cpp
@@ -195,6 +195,14 @@ EventBase::EventBase(event_base* evb, bool enableTimeMeasurement)
 }
 
 EventBase::~EventBase() {
+  // Keep looping until all keep-alive handles are released. Each keep-alive
+  // handle signals that some external code will still schedule some work on
+  // this EventBase (so it's not safe to destroy it).
+  while (!loopKeepAlive_.unique()) {
+    applyLoopKeepAlive();
+    loopOnce();
+  }
+
   // Call all destruction callbacks, before we start cleaning up our state.
   while (!onDestructionCallbacks_.empty()) {
     LoopCallback* callback = &onDestructionCallbacks_.front();

diff --git a/folly/io/async/test/EventBaseTest.cpp b/folly/io/async/test/EventBaseTest.cpp
index 7cb49a4dd70cbbbc7a973593ac9631ed1ef72a47..46840882122045acfb8f5e0de176c4ae96a552f1 100644 (file)
--- a/folly/io/async/test/EventBaseTest.cpp
+++ b/folly/io/async/test/EventBaseTest.cpp
@@ -1736,7 +1736,8 @@ TEST(EventBaseTest, LoopKeepAlive) {
   std::thread t([&, loopKeepAlive = evb.loopKeepAlive() ] {
     /* sleep override */ std::this_thread::sleep_for(
         std::chrono::milliseconds(100));
-    evb.runInEventBaseThread([&] { done = true; });
+    evb.runInEventBaseThread(
+        [&done, loopKeepAlive = std::move(loopKeepAlive) ] { done = true; });
   });
 
   evb.loop();
@@ -1756,7 +1757,8 @@ TEST(EventBaseTest, LoopKeepAliveInLoop) {
     t = std::thread([&, loopKeepAlive = evb.loopKeepAlive() ] {
       /* sleep override */ std::this_thread::sleep_for(
           std::chrono::milliseconds(100));
-      evb.runInEventBaseThread([&] { done = true; });
+      evb.runInEventBaseThread(
+          [&done, loopKeepAlive = std::move(loopKeepAlive) ] { done = true; });
     });
   });
 
@@ -1766,3 +1768,25 @@ TEST(EventBaseTest, LoopKeepAliveInLoop) {
 
   t.join();
 }
+
+TEST(EventBaseTest, LoopKeepAliveShutdown) {
+  auto evb = folly::make_unique<EventBase>();
+
+  bool done = false;
+
+  std::thread t(
+      [&done, loopKeepAlive = evb->loopKeepAlive(), evbPtr = evb.get() ] {
+        /* sleep override */ std::this_thread::sleep_for(
+            std::chrono::milliseconds(100));
+        evbPtr->runInEventBaseThread(
+            [&done, loopKeepAlive = std::move(loopKeepAlive) ] {
+              done = true;
+            });
+      });
+
+  evb.reset();
+
+  ASSERT_TRUE(done);
+
+  t.join();
+}