Commit 79ae7831 authored by Alecs King's avatar Alecs King Committed by Noam Lerner

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
parent 789dbd17
...@@ -13,21 +13,49 @@ ...@@ -13,21 +13,49 @@
* See the License for the specific language governing permissions and * See the License for the specific language governing permissions and
* limitations under the License. * limitations under the License.
*/ */
#include <folly/experimental/fibers/FiberManagerMap.h> #include "FiberManagerMap.h"
#include <cassert>
#include <memory> #include <memory>
#include <unordered_map> #include <unordered_map>
#include <folly/ThreadLocal.h>
namespace folly { namespace fibers { namespace folly { namespace fibers {
namespace detail { namespace detail {
thread_local std::unordered_map<folly::EventBase*, FiberManager*> class LocalFiberManagerMapTag;
localFiberManagerMap; folly::ThreadLocal<std::unordered_map<folly::EventBase*, FiberManager*>,
LocalFiberManagerMapTag> localFiberManagerMap;
std::unordered_map<folly::EventBase*, std::unique_ptr<FiberManager>> std::unordered_map<folly::EventBase*, std::unique_ptr<FiberManager>>
fiberManagerMap; fiberManagerMap;
std::mutex fiberManagerMapMutex; 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, FiberManager* getFiberManagerThreadSafe(folly::EventBase& evb,
const FiberManager::Options& opts) { const FiberManager::Options& opts) {
std::lock_guard<std::mutex> lg(fiberManagerMapMutex); std::lock_guard<std::mutex> lg(fiberManagerMapMutex);
...@@ -42,6 +70,7 @@ FiberManager* getFiberManagerThreadSafe(folly::EventBase& evb, ...@@ -42,6 +70,7 @@ FiberManager* getFiberManagerThreadSafe(folly::EventBase& evb,
auto fiberManager = auto fiberManager =
folly::make_unique<FiberManager>(std::move(loopController), opts); folly::make_unique<FiberManager>(std::move(loopController), opts);
auto result = fiberManagerMap.emplace(&evb, std::move(fiberManager)); auto result = fiberManagerMap.emplace(&evb, std::move(fiberManager));
evb.runOnDestruction(new OnEventBaseDestructionCallback(evb));
return result.first->second.get(); return result.first->second.get();
} }
...@@ -49,13 +78,14 @@ FiberManager* getFiberManagerThreadSafe(folly::EventBase& evb, ...@@ -49,13 +78,14 @@ FiberManager* getFiberManagerThreadSafe(folly::EventBase& evb,
FiberManager& getFiberManager(folly::EventBase& evb, FiberManager& getFiberManager(folly::EventBase& evb,
const FiberManager::Options& opts) { const FiberManager::Options& opts) {
auto it = detail::localFiberManagerMap.find(&evb); auto it = detail::localFiberManagerMap->find(&evb);
if (LIKELY(it != detail::localFiberManagerMap.end())) { if (LIKELY(it != detail::localFiberManagerMap->end())) {
return *(it->second); return *(it->second);
} }
return *(detail::localFiberManagerMap[&evb] = auto fm = detail::getFiberManagerThreadSafe(evb, opts);
detail::getFiberManagerThreadSafe(evb, opts)); detail::localFiberManagerMap->emplace(&evb, fm);
return *fm;
} }
}} }}
...@@ -521,7 +521,7 @@ void EventBase::runInLoop(Cob&& cob, bool thisIteration) { ...@@ -521,7 +521,7 @@ void EventBase::runInLoop(Cob&& cob, bool thisIteration) {
} }
void EventBase::runOnDestruction(LoopCallback* callback) { void EventBase::runOnDestruction(LoopCallback* callback) {
DCHECK(isInEventBaseThread()); std::lock_guard<std::mutex> lg(onDestructionCallbacksMutex_);
callback->cancelLoopCallback(); callback->cancelLoopCallback();
onDestructionCallbacks_.push_back(*callback); onDestructionCallbacks_.push_back(*callback);
} }
......
...@@ -725,6 +725,9 @@ bool runImmediatelyOrRunInEventBaseThreadAndWait(const Cob& fn); ...@@ -725,6 +725,9 @@ bool runImmediatelyOrRunInEventBaseThreadAndWait(const Cob& fn);
// Name of the thread running this EventBase // Name of the thread running this EventBase
std::string name_; std::string name_;
// allow runOnDestruction() to be called from any threads
std::mutex onDestructionCallbacksMutex_;
}; };
} // folly } // folly
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment