From: Alecs King <int@fb.com>
Date: Wed, 3 Jun 2015 00:04:25 +0000 (-0700)
Subject: fix mem leak
X-Git-Tag: v0.42.0~3
X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=79ae78317bb26ac56fa2feeeb0292935f36c85a5;p=folly.git

fix mem leak

Summary:
- use folly::ThreadLocal to work around GCC bug 57914 (with the benefit of accessAllThreads)
- clean up corresponding thread-local and global cache entries before eventbase gets destructed since there was a possible memory leak for short-term living eventbase.

Test Plan:
tests

Reviewed By: andrii@fb.com

Subscribers: smarlow, rushix, ilyam, trunkagent, folly-diffs@, yfeldblum, chalfant, jinfu

FB internal diff: D2116216

Tasks: 7291028, 7279391

Signature: t1:2116216:1433212893:e57a7df90b15b89ccd9471469e669c6e7dc477bf

Blame Revision: D1941662
---

diff --git a/folly/experimental/fibers/FiberManagerMap.cpp b/folly/experimental/fibers/FiberManagerMap.cpp
index 0f3e441b..355facbc 100644
--- a/folly/experimental/fibers/FiberManagerMap.cpp
+++ b/folly/experimental/fibers/FiberManagerMap.cpp
@@ -13,21 +13,49 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#include <folly/experimental/fibers/FiberManagerMap.h>
+#include "FiberManagerMap.h"
 
+#include <cassert>
 #include <memory>
 #include <unordered_map>
 
+#include <folly/ThreadLocal.h>
+
 namespace folly { namespace fibers {
 
 namespace detail {
 
-thread_local std::unordered_map<folly::EventBase*, FiberManager*>
-    localFiberManagerMap;
+class LocalFiberManagerMapTag;
+folly::ThreadLocal<std::unordered_map<folly::EventBase*, FiberManager*>,
+  LocalFiberManagerMapTag> localFiberManagerMap;
 std::unordered_map<folly::EventBase*, std::unique_ptr<FiberManager>>
     fiberManagerMap;
 std::mutex fiberManagerMapMutex;
 
+class OnEventBaseDestructionCallback : public folly::EventBase::LoopCallback {
+ public:
+  explicit OnEventBaseDestructionCallback(folly::EventBase& evb)
+           : evb_(&evb) {}
+  void runLoopCallback() noexcept override {
+    for (auto& localMap : localFiberManagerMap.accessAllThreads()) {
+      localMap.erase(evb_);
+    }
+    std::unique_ptr<FiberManager> fm;
+    {
+      std::lock_guard<std::mutex> lg(fiberManagerMapMutex);
+      auto it = fiberManagerMap.find(evb_);
+      assert(it != fiberManagerMap.end());
+      fm = std::move(it->second);
+      fiberManagerMap.erase(it);
+    }
+    assert(fm.get() != nullptr);
+    fm->loopUntilNoReady();
+    delete this;
+  }
+ private:
+  folly::EventBase* evb_;
+};
+
 FiberManager* getFiberManagerThreadSafe(folly::EventBase& evb,
                                         const FiberManager::Options& opts) {
   std::lock_guard<std::mutex> lg(fiberManagerMapMutex);
@@ -42,6 +70,7 @@ FiberManager* getFiberManagerThreadSafe(folly::EventBase& evb,
   auto fiberManager =
       folly::make_unique<FiberManager>(std::move(loopController), opts);
   auto result = fiberManagerMap.emplace(&evb, std::move(fiberManager));
+  evb.runOnDestruction(new OnEventBaseDestructionCallback(evb));
   return result.first->second.get();
 }
 
@@ -49,13 +78,14 @@ FiberManager* getFiberManagerThreadSafe(folly::EventBase& evb,
 
 FiberManager& getFiberManager(folly::EventBase& evb,
                               const FiberManager::Options& opts) {
-  auto it = detail::localFiberManagerMap.find(&evb);
-  if (LIKELY(it != detail::localFiberManagerMap.end())) {
+  auto it = detail::localFiberManagerMap->find(&evb);
+  if (LIKELY(it != detail::localFiberManagerMap->end())) {
     return *(it->second);
   }
 
-  return *(detail::localFiberManagerMap[&evb] =
-               detail::getFiberManagerThreadSafe(evb, opts));
+  auto fm = detail::getFiberManagerThreadSafe(evb, opts);
+  detail::localFiberManagerMap->emplace(&evb, fm);
+  return *fm;
 }
 
 }}
diff --git a/folly/io/async/EventBase.cpp b/folly/io/async/EventBase.cpp
index a3e5bdc9..d9016506 100644
--- a/folly/io/async/EventBase.cpp
+++ b/folly/io/async/EventBase.cpp
@@ -521,7 +521,7 @@ void EventBase::runInLoop(Cob&& cob, bool thisIteration) {
 }
 
 void EventBase::runOnDestruction(LoopCallback* callback) {
-  DCHECK(isInEventBaseThread());
+  std::lock_guard<std::mutex> lg(onDestructionCallbacksMutex_);
   callback->cancelLoopCallback();
   onDestructionCallbacks_.push_back(*callback);
 }
diff --git a/folly/io/async/EventBase.h b/folly/io/async/EventBase.h
index 843ab464..9077b06c 100644
--- a/folly/io/async/EventBase.h
+++ b/folly/io/async/EventBase.h
@@ -725,6 +725,9 @@ bool runImmediatelyOrRunInEventBaseThreadAndWait(const Cob& fn);
 
   // Name of the thread running this EventBase
   std::string name_;
+
+  // allow runOnDestruction() to be called from any threads
+  std::mutex onDestructionCallbacksMutex_;
 };
 
 } // folly