From e5cb9b3d4ce31408239c2d316520ddcc4758a22e Mon Sep 17 00:00:00 2001 From: Yedidya Feldblum Date: Fri, 16 Dec 2016 20:20:35 -0800 Subject: [PATCH] Split EventBaseThread from ScopedEventBaseThread Summary: [Folly] Split `EventBaseThread` from `ScopedEventBaseThread`. Now `ScopedEventBaseThread` is really scoped and immovable, while `EventBaseThread` is movable and can be started and stopped. Users which will never move, and will never start or stop, the `ScopedEventBaseThread` can continue using it. Users which need to move, or which need to start and stop, the object will use `EventBaseThread` instead. Reviewed By: andriigrynenko Differential Revision: D4338447 fbshipit-source-id: 57c186630bc199a7a7b7223b1fcb077ce3d86743 --- folly/Makefile.am | 2 + folly/io/async/EventBaseThread.cpp | 60 ++++++++++++ folly/io/async/EventBaseThread.h | 50 ++++++++++ folly/io/async/ScopedEventBaseThread.cpp | 53 +++------- folly/io/async/ScopedEventBaseThread.h | 29 ++---- folly/io/async/test/EventBaseThreadTest.cpp | 98 +++++++++++++++++++ .../async/test/ScopedEventBaseThreadTest.cpp | 45 +-------- 7 files changed, 233 insertions(+), 104 deletions(-) create mode 100644 folly/io/async/EventBaseThread.cpp create mode 100644 folly/io/async/EventBaseThread.h create mode 100644 folly/io/async/test/EventBaseThreadTest.cpp diff --git a/folly/Makefile.am b/folly/Makefile.am index bb87d512..19cf569b 100644 --- a/folly/Makefile.am +++ b/folly/Makefile.am @@ -223,6 +223,7 @@ nobase_follyinclude_HEADERS = \ io/async/EventBase.h \ io/async/EventBaseLocal.h \ io/async/EventBaseManager.h \ + io/async/EventBaseThread.h \ io/async/EventFDWrapper.h \ io/async/EventHandler.h \ io/async/EventUtil.h \ @@ -450,6 +451,7 @@ libfolly_la_SOURCES = \ io/async/EventBase.cpp \ io/async/EventBaseLocal.cpp \ io/async/EventBaseManager.cpp \ + io/async/EventBaseThread.cpp \ io/async/EventHandler.cpp \ io/async/Request.cpp \ io/async/SSLContext.cpp \ diff --git a/folly/io/async/EventBaseThread.cpp b/folly/io/async/EventBaseThread.cpp new file mode 100644 index 00000000..ebe9ac50 --- /dev/null +++ b/folly/io/async/EventBaseThread.cpp @@ -0,0 +1,60 @@ +/* + * Copyright 2016 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include + +namespace folly { + +EventBaseThread::EventBaseThread() : EventBaseThread(true) {} + +EventBaseThread::EventBaseThread(bool autostart, EventBaseManager* ebm) + : ebm_(ebm) { + if (autostart) { + start(); + } +} + +EventBaseThread::EventBaseThread(EventBaseManager* ebm) + : EventBaseThread(true, ebm) {} + +EventBaseThread::~EventBaseThread() = default; + +EventBaseThread::EventBaseThread(EventBaseThread&&) noexcept = default; +EventBaseThread& EventBaseThread::operator=(EventBaseThread&&) noexcept = + default; + +EventBase* EventBaseThread::getEventBase() const { + return th_ ? th_->getEventBase() : nullptr; +} + +bool EventBaseThread::running() const { + return !!th_; +} + +void EventBaseThread::start() { + if (th_) { + return; + } + th_ = make_unique(ebm_); +} + +void EventBaseThread::stop() { + th_ = nullptr; +} +} diff --git a/folly/io/async/EventBaseThread.h b/folly/io/async/EventBaseThread.h new file mode 100644 index 00000000..a4ac541a --- /dev/null +++ b/folly/io/async/EventBaseThread.h @@ -0,0 +1,50 @@ +/* + * Copyright 2016 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +namespace folly { + +class EventBase; +class EventBaseManager; +class ScopedEventBaseThread; + +class EventBaseThread { + public: + EventBaseThread(); + explicit EventBaseThread(bool autostart, EventBaseManager* ebm = nullptr); + explicit EventBaseThread(EventBaseManager* ebm); + ~EventBaseThread(); + + EventBaseThread(EventBaseThread&&) noexcept; + EventBaseThread& operator=(EventBaseThread&&) noexcept; + + EventBase* getEventBase() const; + + bool running() const; + void start(); + void stop(); + + private: + EventBaseThread(EventBaseThread const&) = default; + EventBaseThread& operator=(EventBaseThread const&) = default; + + EventBaseManager* ebm_; + std::unique_ptr th_; +}; +} diff --git a/folly/io/async/ScopedEventBaseThread.cpp b/folly/io/async/ScopedEventBaseThread.cpp index 29fa4522..1e78f5ee 100644 --- a/folly/io/async/ScopedEventBaseThread.cpp +++ b/folly/io/async/ScopedEventBaseThread.cpp @@ -17,7 +17,9 @@ #include #include + #include +#include using namespace std; @@ -25,55 +27,22 @@ namespace folly { static void run(EventBaseManager* ebm, EventBase* eb) { ebm->setEventBase(eb, false); - CHECK_NOTNULL(eb)->loopForever(); + eb->loopForever(); ebm->clearEventBase(); } -ScopedEventBaseThread::ScopedEventBaseThread( - bool autostart, - EventBaseManager* ebm) +ScopedEventBaseThread::ScopedEventBaseThread() + : ScopedEventBaseThread(nullptr) {} + +ScopedEventBaseThread::ScopedEventBaseThread(EventBaseManager* ebm) : ebm_(ebm ? ebm : EventBaseManager::get()) { - if (autostart) { - start(); - } + th_ = thread(run, ebm_, &eb_); + eb_.waitUntilRunning(); } -ScopedEventBaseThread::ScopedEventBaseThread( - EventBaseManager* ebm) : - ScopedEventBaseThread(true, ebm) {} - ScopedEventBaseThread::~ScopedEventBaseThread() { - stop(); -} - -ScopedEventBaseThread::ScopedEventBaseThread( - ScopedEventBaseThread&& /* other */) noexcept = default; - -ScopedEventBaseThread& ScopedEventBaseThread::operator=( - ScopedEventBaseThread&& /* other */) noexcept = default; - -void ScopedEventBaseThread::start() { - if (running()) { - return; - } - eventBase_ = make_unique(); - thread_ = make_unique(run, ebm_, eventBase_.get()); - eventBase_->waitUntilRunning(); -} - -void ScopedEventBaseThread::stop() { - if (!running()) { - return; - } - eventBase_->terminateLoopSoon(); - thread_->join(); - eventBase_ = nullptr; - thread_ = nullptr; -} - -bool ScopedEventBaseThread::running() { - CHECK(bool(eventBase_) == bool(thread_)); - return eventBase_ && thread_; + eb_.terminateLoopSoon(); + th_.join(); } } diff --git a/folly/io/async/ScopedEventBaseThread.h b/folly/io/async/ScopedEventBaseThread.h index 0370eb9b..c59b275e 100644 --- a/folly/io/async/ScopedEventBaseThread.h +++ b/folly/io/async/ScopedEventBaseThread.h @@ -19,10 +19,11 @@ #include #include #include -#include namespace folly { +class EventBaseManager; + /** * A helper class to start a new thread running a EventBase loop. * @@ -32,34 +33,24 @@ namespace folly { */ class ScopedEventBaseThread { public: - explicit ScopedEventBaseThread( - bool autostart = true, - EventBaseManager* ebm = nullptr); - explicit ScopedEventBaseThread( - EventBaseManager* ebm); + ScopedEventBaseThread(); + explicit ScopedEventBaseThread(EventBaseManager* ebm); ~ScopedEventBaseThread(); - ScopedEventBaseThread(ScopedEventBaseThread&& other) noexcept; - ScopedEventBaseThread &operator=(ScopedEventBaseThread&& other) noexcept; - - /** - * Get a pointer to the EventBase driving this thread. - */ EventBase* getEventBase() const { - return eventBase_.get(); + return &eb_; } - void start(); - void stop(); - bool running(); - private: + ScopedEventBaseThread(ScopedEventBaseThread&& other) = delete; + ScopedEventBaseThread& operator=(ScopedEventBaseThread&& other) = delete; + ScopedEventBaseThread(const ScopedEventBaseThread& other) = delete; ScopedEventBaseThread& operator=(const ScopedEventBaseThread& other) = delete; EventBaseManager* ebm_; - std::unique_ptr eventBase_; - std::unique_ptr thread_; + mutable EventBase eb_; + std::thread th_; }; } diff --git a/folly/io/async/test/EventBaseThreadTest.cpp b/folly/io/async/test/EventBaseThreadTest.cpp new file mode 100644 index 00000000..0675929a --- /dev/null +++ b/folly/io/async/test/EventBaseThreadTest.cpp @@ -0,0 +1,98 @@ +/* + * Copyright 2016 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +#include +#include +#include + +using namespace std; +using namespace std::chrono; +using namespace folly; + +class EventBaseThreadTest : public testing::Test {}; + +TEST_F(EventBaseThreadTest, example) { + EventBaseThread ebt; + + Baton<> done; + ebt.getEventBase()->runInEventBaseThread([&] { done.post(); }); + ASSERT_TRUE(done.timed_wait(seconds(1))); +} + +TEST_F(EventBaseThreadTest, start_stop) { + EventBaseThread ebt(false); + + for (size_t i = 0; i < 4; ++i) { + EXPECT_EQ(nullptr, ebt.getEventBase()); + ebt.start(); + EXPECT_NE(nullptr, ebt.getEventBase()); + + Baton<> done; + ebt.getEventBase()->runInEventBaseThread([&] { done.post(); }); + ASSERT_TRUE(done.timed_wait(seconds(1))); + + EXPECT_NE(nullptr, ebt.getEventBase()); + ebt.stop(); + EXPECT_EQ(nullptr, ebt.getEventBase()); + } +} + +TEST_F(EventBaseThreadTest, move) { + auto ebt0 = EventBaseThread(); + auto ebt1 = std::move(ebt0); + auto ebt2 = std::move(ebt1); + + EXPECT_EQ(nullptr, ebt0.getEventBase()); + EXPECT_EQ(nullptr, ebt1.getEventBase()); + EXPECT_NE(nullptr, ebt2.getEventBase()); + + Baton<> done; + ebt2.getEventBase()->runInEventBaseThread([&] { done.post(); }); + ASSERT_TRUE(done.timed_wait(seconds(1))); +} + +TEST_F(EventBaseThreadTest, self_move) { + EventBaseThread ebt0; + auto ebt = std::move(ebt0); + + EXPECT_NE(nullptr, ebt.getEventBase()); + + Baton<> done; + ebt.getEventBase()->runInEventBaseThread([&] { done.post(); }); + ASSERT_TRUE(done.timed_wait(seconds(1))); +} + +TEST_F(EventBaseThreadTest, default_manager) { + auto ebm = EventBaseManager::get(); + EventBaseThread ebt; + auto ebt_eb = ebt.getEventBase(); + auto ebm_eb = static_cast(nullptr); + ebt_eb->runInEventBaseThreadAndWait([&] { ebm_eb = ebm->getEventBase(); }); + EXPECT_EQ(uintptr_t(ebt_eb), uintptr_t(ebm_eb)); +} + +TEST_F(EventBaseThreadTest, custom_manager) { + EventBaseManager ebm; + EventBaseThread ebt(&ebm); + auto ebt_eb = ebt.getEventBase(); + auto ebm_eb = static_cast(nullptr); + ebt_eb->runInEventBaseThreadAndWait([&] { ebm_eb = ebm.getEventBase(); }); + EXPECT_EQ(uintptr_t(ebt_eb), uintptr_t(ebm_eb)); +} diff --git a/folly/io/async/test/ScopedEventBaseThreadTest.cpp b/folly/io/async/test/ScopedEventBaseThreadTest.cpp index 6d7ba4c1..f9ed2bf8 100644 --- a/folly/io/async/test/ScopedEventBaseThreadTest.cpp +++ b/folly/io/async/test/ScopedEventBaseThreadTest.cpp @@ -17,7 +17,9 @@ #include #include + #include +#include #include using namespace std; @@ -34,49 +36,6 @@ TEST_F(ScopedEventBaseThreadTest, example) { ASSERT_TRUE(done.timed_wait(seconds(1))); } -TEST_F(ScopedEventBaseThreadTest, start_stop) { - ScopedEventBaseThread sebt(false); - - for (size_t i = 0; i < 4; ++i) { - EXPECT_EQ(nullptr, sebt.getEventBase()); - sebt.start(); - EXPECT_NE(nullptr, sebt.getEventBase()); - - Baton<> done; - sebt.getEventBase()->runInEventBaseThread([&] { done.post(); }); - ASSERT_TRUE(done.timed_wait(seconds(1))); - - EXPECT_NE(nullptr, sebt.getEventBase()); - sebt.stop(); - EXPECT_EQ(nullptr, sebt.getEventBase()); - } -} - -TEST_F(ScopedEventBaseThreadTest, move) { - auto sebt0 = ScopedEventBaseThread(); - auto sebt1 = std::move(sebt0); - auto sebt2 = std::move(sebt1); - - EXPECT_EQ(nullptr, sebt0.getEventBase()); - EXPECT_EQ(nullptr, sebt1.getEventBase()); - EXPECT_NE(nullptr, sebt2.getEventBase()); - - Baton<> done; - sebt2.getEventBase()->runInEventBaseThread([&] { done.post(); }); - ASSERT_TRUE(done.timed_wait(seconds(1))); -} - -TEST_F(ScopedEventBaseThreadTest, self_move) { - ScopedEventBaseThread sebt0; - auto sebt = std::move(sebt0); - - EXPECT_NE(nullptr, sebt.getEventBase()); - - Baton<> done; - sebt.getEventBase()->runInEventBaseThread([&] { done.post(); }); - ASSERT_TRUE(done.timed_wait(seconds(1))); -} - TEST_F(ScopedEventBaseThreadTest, default_manager) { auto ebm = EventBaseManager::get(); ScopedEventBaseThread sebt; -- 2.34.1