Memory leak detected when creating threads in libc++

[bhupeshpant19jan]

---

Memory leak is detected when we are creating std::thread object.

Our existing test suite has started detecting memory leak where we are creating std::thread object after we moved from gnustl to libc++. Apart from stdlib changes, nothing has changed in our code.

I also wrote a test case which to replicate the issue.

TEST_METHOD(Test_make_thread)
{
               std::thread thread_to_test(foo);
		thread_to_test.join();
		TestAssert::IsFalse(false);
}	

I am seeing this issue during teardown process.
* FAIL:
4 leaks

[1 / 4] Leaked 4 bytes allocated at 0xabc29f88
 at Mso::Memory::AllocateEx(unsigned int, unsigned long) (line 0)
 at Mso::Memory::Throw::AllocateEx(unsigned int, unsigned long) (line 0)
 at operator new(unsigned int) (line 0)
 at unknown
 at Mso::Events::UnitTest::ResetableWaitableEventTests::Test_make_thread() (line 0)
 at RunnableMotifTest::Run(void (MotifCppTestBase::*)(), bool) (line 0)
 at RunnableMotifTest::RunTest() (line 0)
 at UnitTestRunner::RunTest(RunnableMotifTest*, TestErrorLogger&) (line 0)
 at UnitTestCollection::TestActionRunTest(UnitTestRunner*, RunnableMotifTest*, TestErrorLogger&) (line 0)
 at unknown
 at unknown
 at unknown
 at unknown
 at UnitTestCollection::TryInvokeTestAction(UnitTestRunner*, RunnableMotifTest*, void (*)(UnitTestRunner*, RunnableMotifTest*, TestErrorLogger&)) (line 0)
 at UnitTestCollection::RunTestAction(RunnableMotifTest*, void (*)(UnitTestRunner*, RunnableMotifTest*, TestErrorLogger&)) (line 0)
 at UnitTestCollection::RunUnitTestMethodImplementation(UnitTestString const&, UnitTestString const&) (line 0)
 at UnitTestCollection::RunUnitTestMethod(UnitTestString const&, UnitTestString const&) (line 0)
 at UnitTestApiRunTestMethodBody(wchar_t*, wchar_t*, tagNativeTestInteropMethods) (line 0)
 at Motif::NativeTestAdapter::TestMethodRun(wchar_t const*, wchar_t const*) (line 0)
 at Motif::NativeMethodRunner::RunTestMethod(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, std::__nd (line 0)
 at Motif::Application::HandleTestMethodInvoke(Motif::TestMethodInvokeRequest const&, long long) (line 0)
 at unknown
 at unknown
 at unknown
 at std::__ndk1::function<void (Motif::TestMethodInvokeRequest, long long)>::operator()(Motif::TestMethodInvokeRequest, long long) const (line 0)
 at void Motif::RegisterReceiver<Motif::TestMethodInvokeRequest>(std::__ndk1::unique_ptr<Grf::Messaging::RequestResponseBroker, std::__ndk1::function<void (Grf::Messaging::RequestResponseBroker*)> >&, std::__ndk1::shared_ptr<Grf::IDispatcher> const&, std::__nd (line 0)
 at Grf::FuncAction<void Motif::RegisterReceiver<Motif::TestMethodInvokeRequest>(std::__ndk1::unique_ptr<Grf::Messaging::RequestResponseBroker, std::__ndk1::function<void (Grf::Messaging::RequestResponseBroker*)> >&, std::__ndk1::shared_ptr<Grf::IDispatcher> c (line 0)
 at void Grf::Messaging::RequestResponseBroker::AddTypedReceiver<Motif::GrfSerializer, Motif::TestMethodInvokeRequest, std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > >(std::__ndk1::map<std::__ndk1::basic_string<c (line 0)
 at void Grf::Details::DeferredFunctionCall<0u>::Call<void Grf::Messaging::RequestResponseBroker::AddTypedReceiver<Motif::GrfSerializer, Motif::TestMethodInvokeRequest, std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char (line 0)
 at void Grf::Details::DeferredFunctionCall<1u>::Call<void Grf::Messaging::RequestResponseBroker::AddTypedReceiver<Motif::GrfSerializer, Motif::TestMethodInvokeRequest, std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char (line 0)
 at void Grf::Details::DeferredFunctionCall<2u>::Call<void Grf::Messaging::RequestResponseBroker::AddTypedReceiver<Motif::GrfSerializer, Motif::TestMethodInvokeRequest, std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char (line 0)
 at Grf::DeferredFunction<void Grf::Messaging::RequestResponseBroker::AddTypedReceiver<Motif::GrfSerializer, Motif::TestMethodInvokeRequest, std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > >(std::__ndk1::map<std:: (line 0)

OAUTOBUGTAG:CLS[DEBUGHEAP_LEAK][4][][][]ENDOABT

Bug reports with better test cases will be resolved sooner than those with bad
test cases. A good test case:

Environment Details

Not all of these will be relevant to every bug, but please provide as much
information as you can.

• NDK Version: r16
• Build system: Clang
• Host OS: Android on Windows
• Compiler: Clang
• ABI:
• STL: libc++
• NDK API level: 21
• Device API level:

[DanAlbert]

Do you see this with r18?

[bhupeshpant19jan]

No, as mentioned above we are using r16.

[DanAlbert]

I see that, but the bug template states:

Before filing a bug, check that you are using the latest version of the NDK.

We don't backport fixes to old NDKs, so if it doesn't repro on r18 there's nothing for us to fix.

[bhupeshpant19jan]

Thanks, Dan for your response.
It won't be straightforward to repro this because our current infra has r16 support, including our test suite. It is not an easy effort to upgrade to r18.
Can you recommend any other tool where I can test this?
Also, is it possible for you to confirm/test this behavior in r16?

[bhupeshpant19jan]

One more update, I have tested the same scenario with gnustl lib and there I am not observing any leak.

Test_make_thread.................................................................................................................................................................................................................PASSED

Top 3 Slowest Tests Seconds

TestFileSetup 00.049647
ResetableWaitableEventTests.Teardown 00.042814
TestFileTeardown 00.042519

Test Method Results

PASSED 2
FAILED 0
WARNINGS 5

Detailed logs are available at "f:\stl\Logs\ut.droidx86.debug.log".

TESTS PASSED

Execution Times

Test Run Time 20 sec
Total Execution Time 20 sec

f:\stl\dev\liblet\events\test>

[DanAlbert]

I'm not sure how I'm going to repro it with any NDK release if you don't have a repro case that works outside your environment. I see there's a snippet in the original post, but it's not complete. What test library is that? How is it built? How are you checking for leaks?

[bhupeshpant19jan]

This is our internal test suite. I am not sure how we are checking for leaks but I strongly think that it is being done in a standard way by overloading new,

[DanAlbert]

Try to reduce a test case if you can.

[bhupeshpant19jan]

I got this link:- http://clang-developers.42468.n3.nabble.com/Libcxx-TizenRT-Memory-leak-in-std-thread-td4060726.html
Which has observed the similar behavior, can you deduce anything with this information?

[bhupeshpant19jan]

Try to reduce a test case if you can.

I am able to reproduce it even with the one test case. I have added others just to try other scenarios.

[stephenhines]

Based on https://github.com/llvm-mirror/libcxx/blob/master/include/thread#L189 (linked directly from that bug), this is an intentional leak. Issue #789 also talks about this.

[bhupeshpant19jan]

template
__thread_specific_ptr<_Tp>::~__thread_specific_ptr()
{
// __thread_specific_ptr is only created with a static storage duration
// so this destructor is only invoked during program termination. Invoking
// pthread_key_delete(_key) may prevent other threads from deleting their
// thread local data. For this reason we leak the key.
}

What should we do about it?
I think I need to understand why are we doing this and how this will impact our applications.

[DanAlbert]

The thread you linked explains the rationale.

This is a common technique used in language libraries and runtimes. It's even considered good practice, at it allows the process to exit faster and eliminates a source of crashes during user-provided atexit() teardown.

And the comment in the code gives another reason:

// __thread_specific_ptr is only created with a static storage duration
// so this destructor is only invoked during program termination. Invoking
// pthread_key_delete(__key_) may prevent other threads from deleting their
// thread local data. For this reason we leak the key.

Closing since this is intended behavior.

[DanAlbert]

As for what you can do about it on your end, I'd probably look at whatever method you're using to detect leaks. Probably would also be informative to see how valgrind or some other leak detector behaves with libc++'s std::thread.

[bhupeshpant19jan]

Appreciate you for your efforts and time for investigating this issue.