Commit 9f502239 authored by Andrii Grynenko's avatar Andrii Grynenko Committed by Sara Golemon

folly::Singleton leak diagnostics utils

Summary: This adds different types of messages depending on whether Singleton was depending on other Singleton or just leaked. It also adds destruction stack trace for such Singletons (if they were ever destroyed) to help debug such leaks/broken dependencies.

Test Plan: unit test

Reviewed By: chip@fb.com

Subscribers: trunkagent, folly-diffs@, yfeldblum

FB internal diff: D1830526

Signature: t1:1830526:1423266462:ba328b0da0bf4030b1c4f686d8f7b609fd20683c
parent 8f7c257f
...@@ -13,6 +13,8 @@ ...@@ -13,6 +13,8 @@
* 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/symbolizer/Symbolizer.h>
namespace folly { namespace folly {
namespace detail { namespace detail {
...@@ -94,6 +96,7 @@ void SingletonHolder<T>::destroyInstance() { ...@@ -94,6 +96,7 @@ void SingletonHolder<T>::destroyInstance() {
auto wait_result = destroy_baton_->timed_wait( auto wait_result = destroy_baton_->timed_wait(
std::chrono::steady_clock::now() + kDestroyWaitTime); std::chrono::steady_clock::now() + kDestroyWaitTime);
if (!wait_result) { if (!wait_result) {
print_destructor_stack_trace_->store(true);
LOG(ERROR) << "Singleton of type " << type_.name() << " has a " LOG(ERROR) << "Singleton of type " << type_.name() << " has a "
<< "living reference at destroyInstances time; beware! Raw " << "living reference at destroyInstances time; beware! Raw "
<< "pointer is " << instance_ptr_ << ". It is very likely " << "pointer is " << instance_ptr_ << ". It is very likely "
...@@ -140,14 +143,42 @@ void SingletonHolder<T>::createInstance() { ...@@ -140,14 +143,42 @@ void SingletonHolder<T>::createInstance() {
} }
auto destroy_baton = std::make_shared<folly::Baton<>>(); auto destroy_baton = std::make_shared<folly::Baton<>>();
auto print_destructor_stack_trace =
std::make_shared<std::atomic<bool>>(false);
auto teardown = teardown_; auto teardown = teardown_;
auto type_name = type_.name();
// Can't use make_shared -- no support for a custom deleter, sadly. // Can't use make_shared -- no support for a custom deleter, sadly.
instance_ = std::shared_ptr<T>( instance_ = std::shared_ptr<T>(
create_(), create_(),
[destroy_baton, teardown](T* instance_ptr) mutable { [destroy_baton, print_destructor_stack_trace, teardown, type_name]
(T* instance_ptr) mutable {
teardown(instance_ptr); teardown(instance_ptr);
destroy_baton->post(); destroy_baton->post();
if (print_destructor_stack_trace->load()) {
std::string output = "Singleton " + type_name + " was destroyed.\n";
// Get and symbolize stack trace
constexpr size_t kMaxStackTraceDepth = 100;
symbolizer::FrameArray<kMaxStackTraceDepth> addresses;
if (!getStackTraceSafe(addresses)) {
output += "Failed to get destructor stack trace.";
} else {
output += "Destructor stack trace:\n";
constexpr size_t kDefaultCapacity = 500;
symbolizer::ElfCache elfCache(kDefaultCapacity);
symbolizer::Symbolizer symbolizer(&elfCache);
symbolizer.symbolize(addresses);
symbolizer::StringSymbolizePrinter printer;
printer.println(addresses);
output += printer.str();
}
LOG(ERROR) << output;
}
}); });
// We should schedule destroyInstances() only after the singleton was // We should schedule destroyInstances() only after the singleton was
...@@ -160,6 +191,7 @@ void SingletonHolder<T>::createInstance() { ...@@ -160,6 +191,7 @@ void SingletonHolder<T>::createInstance() {
instance_ptr_ = instance_.get(); instance_ptr_ = instance_.get();
creating_thread_ = std::thread::id(); creating_thread_ = std::thread::id();
destroy_baton_ = std::move(destroy_baton); destroy_baton_ = std::move(destroy_baton);
print_destructor_stack_trace_ = std::move(print_destructor_stack_trace);
// This has to be the last step, because once state is Living other threads // This has to be the last step, because once state is Living other threads
// may access instance and instance_weak w/o synchronization. // may access instance and instance_weak w/o synchronization.
......
...@@ -48,6 +48,18 @@ void SingletonVault::destroyInstances() { ...@@ -48,6 +48,18 @@ void SingletonVault::destroyInstances() {
++type_iter) { ++type_iter) {
singletons_[*type_iter]->destroyInstance(); singletons_[*type_iter]->destroyInstance();
} }
for (auto& singleton_type: creation_order_) {
auto singleton = singletons_[singleton_type];
if (!singleton->hasLiveInstance()) {
continue;
}
LOG(DFATAL) << "Singleton of type " << singleton->type().name() << " has "
<< "a living reference after destroyInstances was finished;"
<< "beware! It is very likely that this singleton instance "
<< "will be leaked.";
}
} }
{ {
......
...@@ -268,6 +268,8 @@ struct SingletonHolder : public SingletonHolderBase { ...@@ -268,6 +268,8 @@ struct SingletonHolder : public SingletonHolderBase {
CreateFunc create_ = nullptr; CreateFunc create_ = nullptr;
TeardownFunc teardown_ = nullptr; TeardownFunc teardown_ = nullptr;
std::shared_ptr<std::atomic<bool>> print_destructor_stack_trace_;
SingletonHolder(const SingletonHolder&) = delete; SingletonHolder(const SingletonHolder&) = delete;
SingletonHolder& operator=(const SingletonHolder&) = delete; SingletonHolder& operator=(const SingletonHolder&) = delete;
SingletonHolder& operator=(SingletonHolder&&) = delete; SingletonHolder& operator=(SingletonHolder&&) = delete;
......
...@@ -281,6 +281,7 @@ TEST(Singleton, SharedPtrUsage) { ...@@ -281,6 +281,7 @@ TEST(Singleton, SharedPtrUsage) {
auto locked_s1 = weak_s1.lock(); auto locked_s1 = weak_s1.lock();
EXPECT_EQ(locked_s1.get(), s1); EXPECT_EQ(locked_s1.get(), s1);
EXPECT_EQ(shared_s1.use_count(), 2); EXPECT_EQ(shared_s1.use_count(), 2);
LOG(ERROR) << "The following log message with stack trace is expected";
locked_s1.reset(); locked_s1.reset();
EXPECT_EQ(shared_s1.use_count(), 1); EXPECT_EQ(shared_s1.use_count(), 1);
......
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