Let ScopedEventBaseThread destruct the EventBase in the IO thread
authorYedidya Feldblum <yfeldblum@fb.com>
Sat, 17 Dec 2016 08:53:05 +0000 (00:53 -0800)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Sat, 17 Dec 2016 09:03:03 +0000 (01:03 -0800)
Summary: [Folly] Let `ScopedEventBaseThread` destruct the `EventBase` in the IO thread.

Reviewed By: andriigrynenko

Differential Revision: D4330951

fbshipit-source-id: 5b0e7071800e1d49048b08aa1233d72b820fe55d

folly/Makefile.am
folly/io/async/EventBase.h
folly/io/async/ScopedEventBaseThread.cpp
folly/io/async/ScopedEventBaseThread.h
folly/io/async/test/ScopedEventBaseThreadTest.cpp

index 19cf569b346d75c36bde49a77715be4bec471912..c85182400e5b578c73792f75d27c0871b912246c 100644 (file)
@@ -8,6 +8,7 @@ ACLOCAL_AMFLAGS = -I m4
 
 CLEANFILES =
 
+
 noinst_PROGRAMS = generate_fingerprint_tables
 generate_fingerprint_tables_SOURCES = build/GenerateFingerprintTables.cpp
 generate_fingerprint_tables_LDADD = libfollybase.la
index c92e05170fa4b58077b5ba9e81f88c84ed0684e7..9c6f1d7348a9895e3dd6242695ddce06db7d0c33 100644 (file)
@@ -179,6 +179,22 @@ class EventBase : private boost::noncopyable,
     Func function_;
   };
 
+  // Like FunctionLoopCallback, but saves one allocation. Use with caution.
+  //
+  // The caller is responsible for maintaining the lifetime of this callback
+  // until after the point at which the contained function is called.
+  class StackFunctionLoopCallback : public LoopCallback {
+   public:
+    explicit StackFunctionLoopCallback(Func&& function)
+        : function_(std::move(function)) {}
+    void runLoopCallback() noexcept override {
+      Func(std::move(function_))();
+    }
+
+   private:
+    Func function_;
+  };
+
   /**
    * Create a new EventBase object.
    *
index 1e78f5eea031a897882b917ec4a0e243b45c77f9..f22d2af3552bda6cf9b619946bbc00b060ce3541 100644 (file)
@@ -18,6 +18,7 @@
 
 #include <thread>
 
+#include <folly/Function.h>
 #include <folly/Memory.h>
 #include <folly/io/async/EventBaseManager.h>
 
@@ -28,7 +29,11 @@ namespace folly {
 static void run(EventBaseManager* ebm, EventBase* eb) {
   ebm->setEventBase(eb, false);
   eb->loopForever();
-  ebm->clearEventBase();
+
+  // must destruct in io thread for on-destruction callbacks
+  EventBase::StackFunctionLoopCallback cb([=] { ebm->clearEventBase(); });
+  eb->runOnDestruction(&cb);
+  eb->~EventBase();
 }
 
 ScopedEventBaseThread::ScopedEventBaseThread()
@@ -36,6 +41,7 @@ ScopedEventBaseThread::ScopedEventBaseThread()
 
 ScopedEventBaseThread::ScopedEventBaseThread(EventBaseManager* ebm)
     : ebm_(ebm ? ebm : EventBaseManager::get()) {
+  new (&eb_) EventBase();
   th_ = thread(run, ebm_, &eb_);
   eb_.waitUntilRunning();
 }
index c59b275e08e063c67a43294b974752bc8f44d937..4ad23c6bbba2314163612578450bcb3fe606355d 100644 (file)
@@ -18,6 +18,7 @@
 
 #include <memory>
 #include <thread>
+
 #include <folly/io/async/EventBase.h>
 
 namespace folly {
@@ -41,6 +42,10 @@ class ScopedEventBaseThread {
     return &eb_;
   }
 
+  std::thread::id getThreadId() const {
+    return th_.get_id();
+  }
+
  private:
   ScopedEventBaseThread(ScopedEventBaseThread&& other) = delete;
   ScopedEventBaseThread& operator=(ScopedEventBaseThread&& other) = delete;
@@ -49,7 +54,9 @@ class ScopedEventBaseThread {
   ScopedEventBaseThread& operator=(const ScopedEventBaseThread& other) = delete;
 
   EventBaseManager* ebm_;
-  mutable EventBase eb_;
+  union {
+    mutable EventBase eb_;
+  };
   std::thread th_;
 };
 
index f9ed2bf889d2c95f58a3305bbb6ccbb1566dd5f2..b0871b2b2ad6b6ea591fbf82eb0ae66bef67ee86 100644 (file)
@@ -19,6 +19,7 @@
 #include <chrono>
 
 #include <folly/Baton.h>
+#include <folly/Optional.h>
 #include <folly/io/async/EventBaseManager.h>
 #include <folly/portability/GTest.h>
 
@@ -53,3 +54,17 @@ TEST_F(ScopedEventBaseThreadTest, custom_manager) {
   sebt_eb->runInEventBaseThreadAndWait([&] { ebm_eb = ebm.getEventBase(); });
   EXPECT_EQ(uintptr_t(sebt_eb), uintptr_t(ebm_eb));
 }
+
+TEST_F(ScopedEventBaseThreadTest, eb_dtor_in_io_thread) {
+  Optional<ScopedEventBaseThread> sebt;
+  sebt.emplace();
+  auto const io_thread_id = sebt->getThreadId();
+  EXPECT_NE(this_thread::get_id(), io_thread_id) << "sanity";
+
+  auto const eb = sebt->getEventBase();
+  thread::id eb_dtor_thread_id;
+  eb->runOnDestruction(new EventBase::FunctionLoopCallback(
+      [&] { eb_dtor_thread_id = this_thread::get_id(); }));
+  sebt.clear();
+  EXPECT_EQ(io_thread_id, eb_dtor_thread_id);
+}