From e17fce322760eddc0d598a2044cfd639894d75be Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Thu, 11 May 2017 09:19:42 -0700 Subject: [PATCH] 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 --- folly/io/async/ScopedEventBaseThread.cpp | 11 +++++++++-- folly/io/async/ScopedEventBaseThread.h | 2 ++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/folly/io/async/ScopedEventBaseThread.cpp b/folly/io/async/ScopedEventBaseThread.cpp index 9f14b918..1e21aad7 100644 --- 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 0a8775c6..0c6808e6 100644 --- a/folly/io/async/ScopedEventBaseThread.h +++ b/folly/io/async/ScopedEventBaseThread.h @@ -19,6 +19,7 @@ #include #include +#include #include namespace folly { @@ -65,6 +66,7 @@ class ScopedEventBaseThread { mutable EventBase eb_; }; std::thread th_; + folly::Baton<> stop_; }; } -- 2.34.1