fix destruction race for terminateLoopSoon

Summary:
Calling `EventBase::terminateLoopSoon` from a different thread should be a thread safe
operation when there is a concurrently executing `loopForever`, immediately
followed by `EventBase` destruction.

Today, we first set the stop_ flag to stop the event loop, then post a message to
tell eventlib to stop its event loop. ... but IIUC the stop_ flag is the thing
that makes the `while()` loop to keep going forever. Thus setting it before
message is posted may result in the `loopForever` terminate and underlying
EventBase destroyed before we are able to post a message to eventlib.

The fix is to set `stop_ = true` in loop.

Reviewed By: yfeldblum, andriigrynenko

Differential Revision: D29143212

fbshipit-source-id: f102fbad31653dd7525eff0f70600aa71ae02534

folly/io/async/EventBase.cpp

@@ -544,6 +544,8 @@ void EventBase::bumpHandlingTime() {
 void EventBase::terminateLoopSoon() {
   VLOG(5) << "EventBase(): Received terminateLoopSoon() command.";
 
+  auto keepAlive = getKeepAliveToken(this);
+
   // Set stop to true, so the event loop will know to exit.
   stop_.store(true, std::memory_order_relaxed);
 
@@ -553,7 +555,7 @@ void EventBase::terminateLoopSoon() {
   // receives another event.  Send an empty frame to the notification queue
   // so that the event loop will wake up even if there are no other events.
   try {
-    queue_->putMessage([&] { evb_->eb_event_base_loopbreak(); });
+    queue_->putMessage([] {});
   } catch (...) {
     // putMessage() can only fail when the queue is draining in ~EventBase.
   }

folly/io/async/test/EventBaseLocalTest.cpp

@@ -116,7 +116,7 @@ TEST(EventBaseLocalTest, DestructionOrder) {
 
 TEST(EventBaseLocalTest, DestructorStressTest) {
   std::atomic<folly::EventBase*> currentEvb;
-  folly::Baton<> baton, baton2;
+  folly::Baton<> baton;
   const int kIterations = 10000;
   auto thread1 = std::thread([&] {
     for (int i = 0; i < kIterations; i++) {
@@ -124,8 +124,6 @@ TEST(EventBaseLocalTest, DestructorStressTest) {
       currentEvb = &evb;
       baton.post();
       evb.loopForever();
-      baton2.wait();
-      baton2.reset();
     }
   });
   auto thread2 = std::thread([&] {
@@ -136,7 +134,6 @@ TEST(EventBaseLocalTest, DestructorStressTest) {
       auto evb = currentEvb.exchange(nullptr);
       evb->runInEventBaseThreadAndWait([&] { local.emplace(*evb, 4); });
       evb->terminateLoopSoon();
-      baton2.post();
     }
   });