From 257ee895f921c70ba02b4ed132a51f9c4e2c73ac Mon Sep 17 00:00:00 2001 From: Alexey Maykov Date: Wed, 5 Aug 2015 15:45:21 -0700 Subject: [PATCH] Fixed memory leaks Summary: MyRocks valgrind run was showing memory leaks. The fixes are mostly self-explaining. There is only a single usage of ThreadLocalPtr. Potentially, we may think about replacing this use with thread_local, but it will be a bigger change. Another option to consider is using thread_local instead of __thread in ThreadLocalPtr implementation. This way, tls_ can be stored using std::unique_ptr and no destructor would be required. Test Plan: - make check - MyRocks valgrind run doesn't report leaks Reviewers: rven, sdong Reviewed By: sdong Subscribers: dhruba Differential Revision: https://reviews.facebook.net/D43677 --- util/comparator.cc | 13 +++++++------ util/thread_local.cc | 11 +++++++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/util/comparator.cc b/util/comparator.cc index e6063956b..5e3332737 100644 --- a/util/comparator.cc +++ b/util/comparator.cc @@ -8,6 +8,7 @@ // found in the LICENSE file. See the AUTHORS file for names of contributors. #include +#include #include #include "rocksdb/comparator.h" #include "rocksdb/slice.h" @@ -85,22 +86,22 @@ class ReverseBytewiseComparatorImpl : public BytewiseComparatorImpl { }// namespace static port::OnceType once = LEVELDB_ONCE_INIT; -static const Comparator* bytewise; -static const Comparator* rbytewise; +static std::unique_ptr bytewise; +static std::unique_ptr rbytewise; static void InitModule() { - bytewise = new BytewiseComparatorImpl; - rbytewise= new ReverseBytewiseComparatorImpl; + bytewise.reset(new BytewiseComparatorImpl); + rbytewise.reset(new ReverseBytewiseComparatorImpl); } const Comparator* BytewiseComparator() { port::InitOnce(&once, InitModule); - return bytewise; + return bytewise.get(); } const Comparator* ReverseBytewiseComparator() { port::InitOnce(&once, InitModule); - return rbytewise; + return rbytewise.get(); } } // namespace rocksdb diff --git a/util/thread_local.cc b/util/thread_local.cc index 69f9f6e18..349d5d24e 100644 --- a/util/thread_local.cc +++ b/util/thread_local.cc @@ -137,6 +137,17 @@ ThreadLocalPtr::StaticMeta::StaticMeta() : next_instance_id_(0) { if (pthread_key_create(&pthread_key_, &OnThreadExit) != 0) { abort(); } + + // OnThreadExit is not getting called on the main thread. + // Call through the static destructor mechanism to avoid memory leak. + static struct A { + ~A() { + if (tls_) { + OnThreadExit(tls_); + } + } + } a; + head_.next = &head_; head_.prev = &head_;