From 45d349f07c4873738454c09e3e7b4f3665e81454 Mon Sep 17 00:00:00 2001
From: Yedidya Feldblum <yfeldblum@fb.com>
Date: Sat, 17 Dec 2016 00:53:05 -0800
Subject: [PATCH] Let ScopedEventBaseThread destruct the EventBase in the IO
 thread

Summary: [Folly] Let `ScopedEventBaseThread` destruct the `EventBase` in the IO thread.

Reviewed By: andriigrynenko

Differential Revision: D4330951

fbshipit-source-id: 5b0e7071800e1d49048b08aa1233d72b820fe55d
---
 folly/Makefile.am                                |  1 +
 folly/io/async/EventBase.h                       | 16 ++++++++++++++++
 folly/io/async/ScopedEventBaseThread.cpp         |  8 +++++++-
 folly/io/async/ScopedEventBaseThread.h           |  9 ++++++++-
 .../io/async/test/ScopedEventBaseThreadTest.cpp  | 15 +++++++++++++++
 5 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/folly/Makefile.am b/folly/Makefile.am
index 19cf569b..c8518240 100644
--- a/folly/Makefile.am
+++ b/folly/Makefile.am
@@ -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
diff --git a/folly/io/async/EventBase.h b/folly/io/async/EventBase.h
index c92e0517..9c6f1d73 100644
--- a/folly/io/async/EventBase.h
+++ b/folly/io/async/EventBase.h
@@ -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.
    *
diff --git a/folly/io/async/ScopedEventBaseThread.cpp b/folly/io/async/ScopedEventBaseThread.cpp
index 1e78f5ee..f22d2af3 100644
--- a/folly/io/async/ScopedEventBaseThread.cpp
+++ b/folly/io/async/ScopedEventBaseThread.cpp
@@ -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();
 }
diff --git a/folly/io/async/ScopedEventBaseThread.h b/folly/io/async/ScopedEventBaseThread.h
index c59b275e..4ad23c6b 100644
--- a/folly/io/async/ScopedEventBaseThread.h
+++ b/folly/io/async/ScopedEventBaseThread.h
@@ -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_;
 };
 
diff --git a/folly/io/async/test/ScopedEventBaseThreadTest.cpp b/folly/io/async/test/ScopedEventBaseThreadTest.cpp
index f9ed2bf8..b0871b2b 100644
--- a/folly/io/async/test/ScopedEventBaseThreadTest.cpp
+++ b/folly/io/async/test/ScopedEventBaseThreadTest.cpp
@@ -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);
+}
-- 
2.34.1