Possible memory leaks

[odiszapc]

First of all, thank you for the perfect lib! Promise implementation is good and clean. Looking into the lib deeply.

After running this simple line code using Catch framework with valgrind --leak-check=full --show-leak-kinds=all :

TEST_CASE("Test") {
    promise::Defer promise = promise::newPromise();
}

two "still reachable" warnings was thrown:

=3030== 24 bytes in 1 blocks are still reachable in loss record 1 of 2
==3030==    at 0x4C2F1CA: operator new(unsigned long) (vg_replace_malloc.c:334)
==3030==    by 0x7A3C56: promise::pm_memory_pool* promise::pm_stack_new<promise::pm_memory_pool, unsigned long>(unsigned long&&) (stack.hpp:122)
==3030==    by 0x7A390A: promise::pm_size_allocator<56ul>::get_memory_pool() (allocator.hpp:94)
==3030==    by 0x7A347A: void* promise::pm_allocator::obtain<promise::Promise>() (allocator.hpp:195)
==3030==    by 0x7A2DBD: promise::Promise* promise::pm_new<promise::Promise>() (allocator.hpp:216)
==3030==    by 0x7A2913: promise::newPromise() (promise.hpp:1221)
==3030==    by 0x933DA9: ____C_A_T_C_H____T_E_S_T____8() (promise_test.cpp:126)
==3030==    by 0x8E0089: Catch::FreeFunctionTestCase::invoke() const (catch_test_case_registry_impl.hpp:150)
==3030==    by 0x8BAF1C: Catch::TestCase::invoke() const (catch_test_case_info.hpp:176)
==3030==    by 0x8DEE48: Catch::RunContext::invokeActiveTestCase() (catch_run_context.hpp:367)
==3030==    by 0x8DEBEB: Catch::RunContext::runCurrentTest(std::string&, std::string&) (catch_run_context.hpp:340)
==3030==    by 0x8DD934: Catch::RunContext::runTest(Catch::TestCase const&) (catch_run_context.hpp:131)
==3030==
==3030== 88 bytes in 1 blocks are still reachable in loss record 2 of 2
==3030==    at 0x4C2F8B7: operator new[](unsigned long) (vg_replace_malloc.c:423)
==3030==    by 0x7A1A48: promise::pm_allocator::obtain_impl(promise::pm_memory_pool*, unsigned long) (allocator.hpp:118)
==3030==    by 0x7A348F: void* promise::pm_allocator::obtain<promise::Promise>() (allocator.hpp:196)
==3030==    by 0x7A2DBD: promise::Promise* promise::pm_new<promise::Promise>() (allocator.hpp:216)
==3030==    by 0x7A2913: promise::newPromise() (promise.hpp:1221)
==3030==    by 0x933DA9: ____C_A_T_C_H____T_E_S_T____8() (promise_test.cpp:126)
==3030==    by 0x8E0089: Catch::FreeFunctionTestCase::invoke() const (catch_test_case_registry_impl.hpp:150)
==3030==    by 0x8BAF1C: Catch::TestCase::invoke() const (catch_test_case_info.hpp:176)
==3030==    by 0x8DEE48: Catch::RunContext::invokeActiveTestCase() (catch_run_context.hpp:367)
==3030==    by 0x8DEBEB: Catch::RunContext::runCurrentTest(std::string&, std::string&) (catch_run_context.hpp:340)
==3030==    by 0x8DD934: Catch::RunContext::runTest(Catch::TestCase const&) (catch_run_context.hpp:131)
==3030==    by 0x8B8321: Catch::runTests(Catch::Ptr<Catch::Config> const&) (catch_session.hpp:82)

I guess both point to custom memory allocator:

The first one:

promise::pm_memory_pool* promise::pm_stack_new<promise::pm_memory_pool, unsigned long>(unsigned long&&) (stack.hpp:122)

The second:

promise::pm_allocator::obtain_impl(promise::pm_memory_pool*, unsigned long) (allocator.hpp:118)

In both cases I see memory allocation which are once allocated (fix me if I'm wrong) will never be freed.

I'm not sure this is 100% memory leak, but in more complex production examples instead of "still reachable" I see "possibly lost" or even "definitely lost" warnings pointing to the same lines of code.

Could you please clarify this issue, I saw static thread_local memory pooling logic inside, so maybe we could implement some function whose responsibility would be to clear all the pools manually to make valgrind happy? Like this:

TEST_CASE("Test") {
    promise::Defer promise = promise::newPromise();
    promise::pm_allocator::free_pools();
}

Thank you.

[xhawk18]

Hi, thank you for the testing.

I checked the testing code and no memory leak was found!

Maybe you write your own main.cpp and call exit() from the main ? which may cause the thread local variable not freed.

test.cpp

#define CATCH_CONFIG_MAIN
#include <catch2/catch.hpp>

TEST_CASE("Test"){
    promise::Defer promise = promise::newPromise();
}

compiled by

 g++ -I promise -I Catch2/single_include test.cpp

result after run

valgrind --leak-check=full --show-leak-kinds=all ./a.out

==27275== Memcheck, a memory error detector
==27275== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==27275== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==27275== Command: ./a.out
==27275==
===============================================================================
test cases: 1 | 1 passed
assertions: - none -

==27275==
==27275== HEAP SUMMARY:
==27275==     in use at exit: 0 bytes in 0 blocks
==27275==   total heap usage: 1,784 allocs, 1,784 frees, 318,385 bytes allocated
==27275==
==27275== All heap blocks were freed -- no leaks are possible
==27275==
==27275== For counts of detected and suppressed errors, rerun with: -v
==27275== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

[odiszapc]

Thanks for the feedback! My bad, I tried older relaese (not from master) where no deallocation on thread exit was implemented.

Thank you!