Fix a race on destruction of ScopedEventBaseThread

Summary:
This diff fixes a race that happens on destruction of `ScopedEventBaseThread`.

```
Thread1: ~ScopedEventBaseThread()
Thread1: eb_.terminateLoopSoon() <- preempted just after stop_ = true
Thread2: eb->loopForever() in run(...) exits because stop_ is true
Thread2: ...
Thread2: eb->~EventBase()
Thread1: queue_->putMessage(nullptr) <- accesses destroyed EventBase
```

Reviewed By: yfeldblum

Differential Revision: D5042654

fbshipit-source-id: 95515ed7cde09ff5f125ef121bea86ab3907f98a

diff --git a/folly/io/async/ScopedEventBaseThread.cpp b/folly/io/async/ScopedEventBaseThread.cpp
index 9f14b9186f2e3c3813000be7d969b77ce00371b1..1e21aad79a7c4a21f68cd7c3beb1086bbd4f74bd 100644 (file)
--- a/folly/io/async/ScopedEventBaseThread.cpp
+++ b/folly/io/async/ScopedEventBaseThread.cpp
@@ -27,7 +27,11 @@ using namespace std;
 
 namespace folly {
 
-static void run(EventBaseManager* ebm, EventBase* eb, const StringPiece& name) {
+static void run(
+    EventBaseManager* ebm,
+    EventBase* eb,
+    folly::Baton<>* stop,
+    const StringPiece& name) {
   if (name.size()) {
     folly::setThreadName(name);
   }
@@ -38,6 +42,8 @@ static void run(EventBaseManager* ebm, EventBase* eb, const StringPiece& name) {
   // must destruct in io thread for on-destruction callbacks
   EventBase::StackFunctionLoopCallback cb([=] { ebm->clearEventBase(); });
   eb->runOnDestruction(&cb);
+  // wait until terminateLoopSoon() is complete
+  stop->wait();
   eb->~EventBase();
 }
 
@@ -55,12 +61,13 @@ ScopedEventBaseThread::ScopedEventBaseThread(
     const StringPiece& name)
     : ebm_(ebm ? ebm : EventBaseManager::get()) {
   new (&eb_) EventBase();
-  th_ = thread(run, ebm_, &eb_, name);
+  th_ = thread(run, ebm_, &eb_, &stop_, name);
   eb_.waitUntilRunning();
 }
 
 ScopedEventBaseThread::~ScopedEventBaseThread() {
   eb_.terminateLoopSoon();
+  stop_.post();
   th_.join();
 }
 

diff --git a/folly/io/async/ScopedEventBaseThread.h b/folly/io/async/ScopedEventBaseThread.h
index 0a8775c6cc86276ac6d888f39ca1ac6141153c19..0c6808e63bb42423bfa0c2bf532d8fe812c31b38 100644 (file)
--- a/folly/io/async/ScopedEventBaseThread.h
+++ b/folly/io/async/ScopedEventBaseThread.h
@@ -19,6 +19,7 @@
 #include <memory>
 #include <thread>
 
+#include <folly/Baton.h>
 #include <folly/io/async/EventBase.h>
 
 namespace folly {
@@ -65,6 +66,7 @@ class ScopedEventBaseThread {
     mutable EventBase eb_;
   };
   std::thread th_;
+  folly::Baton<> stop_;
 };
 
 }