Memory leak with std::string depends on string size

Question

I am getting a memory leaking in C++ depending on the string size. On the code bellow if the string size is less than 15 valgrind doesn't shows leak, however if string size is greater than 15 I am getting memory leaks:

#include <string>
#include <memory>
#include <iostream>

class AuthBase {
public:
    virtual void foo() = 0;
};

class APIAuthetication : public AuthBase {
public:
    APIAuthetication(const std::string api_key, const std::string api_secret) : m_api_key(api_key),
                m_api_secret(api_secret) {}
    
    virtual ~APIAuthetication() {}

    void foo() {}
private:
    const std::string m_api_key;
    const std::string m_api_secret;
};

class ClientBase {
public:
    virtual void bar() = 0;
};

class Client: public ClientBase {
public:
    Client(AuthBase* auth) : m_auth(auth) {}
    ~Client() {}

    void bar() {}
private:
    std::unique_ptr<AuthBase> m_auth;
};

class ClientAPI : public Client {
public:
    ClientAPI(const std::string api_key, const std::string api_secret) : 
                Client((new APIAuthetication(api_key, api_secret))) {}
    virtual ~ClientAPI() {}
};

int main(void) {
    /* This runs fine. String size == 15 */
    ClientAPI cli("123456789101112", "123456789101112");
    /* Memory leak. String size > 15 */
    ClientAPI cli_leak("1234567891011121", "1234567891011121");
    return 0;
}

==17060== HEAP SUMMARY:
==17060==     in use at exit: 34 bytes in 2 blocks
==17060==   total heap usage: 9 allocs, 7 frees, 72,950 bytes allocated
==17060== 
==17060== Searching for pointers to 2 not-freed blocks
==17060== Checked 111,624 bytes
==17060== 
==17060== 17 bytes in 1 blocks are definitely lost in loss record 1 of 2
==17060==    at 0x4C3217F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==17060==    by 0x4F6513C: void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*, std::forward_iterator_tag) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==17060==    by 0x1091B3: APIAuthetication::APIAuthetication(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (in /home/rogerio/code-studies/curl-examples/weird)
==17060==    by 0x10937C: ClientAPI::ClientAPI(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (in /home/rogerio/code-studies/curl-examples/weird)
==17060==    by 0x108FD3: main (in /home/rogerio/code-studies/curl-examples/weird)
==17060== 
==17060== 17 bytes in 1 blocks are definitely lost in loss record 2 of 2
==17060==    at 0x4C3217F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==17060==    by 0x4F6513C: void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*, std::forward_iterator_tag) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==17060==    by 0x1091CA: APIAuthetication::APIAuthetication(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (in /home/rogerio/code-studies/curl-examples/weird)
==17060==    by 0x10937C: ClientAPI::ClientAPI(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (in /home/rogerio/code-studies/curl-examples/weird)
==17060==    by 0x108FD3: main (in /home/rogerio/code-studies/curl-examples/weird)
==17060== 
==17060== LEAK SUMMARY:
==17060==    definitely lost: 34 bytes in 2 blocks
==17060==    indirectly lost: 0 bytes in 0 blocks
==17060==      possibly lost: 0 bytes in 0 blocks
==17060==    still reachable: 0 bytes in 0 blocks
==17060==         suppressed: 0 bytes in 0 blocks
==17060== 
==17060== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
==17060== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

No leaks if string size is < 15:

==17211== 
==17211== HEAP SUMMARY:
==17211==     in use at exit: 0 bytes in 0 blocks
==17211==   total heap usage: 2 allocs, 2 frees, 72,776 bytes allocated
==17211== 
==17211== All heap blocks were freed -- no leaks are possible
==17211== 
==17211== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
==17211== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

I know that if a string is less than 16 bytes it doesn't allocate memory and uses a char buffer (optimization), but makes no sense to me this memory leak. Does anyone have an idea what is wrong with my code?

Give this a read valgrind.org/docs/manual/faq.html#faq.reports. I had a similar question a while back and this seemed to be the cause. — Matt
– Matt, Commented Feb 7, 2022 at 4:47
Not a direct answer to your question, but if you're going to be passing std::string around to constructors, make it a const reference. Otherwise you're just creating copies for no reason. — 3Dave
– 3Dave, Commented Feb 7, 2022 at 4:47
As to why it depends on string size: most std::string implementations use so-called small string optimization - there's a small buffer inside std::string object itself, and if the string is short enough, it's stored in this buffer, avoiding heap allocation. Longer strings are allocated on the heap. Naturally, if heap allocation is avoided this way, there's nothing to leak. — Igor Tandetnik
– Igor Tandetnik, Commented Feb 7, 2022 at 4:53
you can set compiler flags / #defines to tell the c++ runtime to release strings correctly. It varied from compiler to compiler — pm100
– pm100, Commented Feb 7, 2022 at 5:42

Remy Lebeau · Accepted Answer · 2022-02-07 05:58:26Z

You already know why the size of the leaks depend on each string's content (short string optimization vs dynamic allocation).

The leaks themselves are because AuthBase does not have a virtual destructor.

When the std::unique_ptr member in Client is destroyed, it will call delete on the AuthBase* pointer it is holding. And since AuthBase's (default-generated) destructor is not virtual, APIAuthetication's destructor will be not called, thus its 2 std::string members will not be destroyed, leaking any memory they have allocated for themselves.

You need to add a virtual destructor to AuthBase:

class AuthBase {
public:
    virtual ~AuthBase() = default;
    ...
};

And then change APIAuthetication's destructor to override that destructor:

class APIAuthetication : public AuthBase {
public:
    ...
    ~APIAuthetication() override = default;
    ...
};

The same goes with your ClientBase class and its derived classes, too.

class ClientBase {
public:
    virtual ~ClientBase() = default;
    ...
};

class Client : public ClientBase {
public:
    ...
    ~Client() override = default;
    ...
};

class ClientAPI : public Client {
public:
    ...
    ~ClientAPI() override = default;
};

Rule of thumb: if a class can be derived from, it should declare (or inherit) a virtual destructor.

On a side note, it is not a good idea for Client's constructor to take a raw AuthBase* pointer as input. It should take a std::unique_ptr<AuthBase> instead. In modern C++ coding, it is best to stay away from calling new/delete directly whenever possible. Let smart pointers do their job cradle-to-grave, so to speak.

class Client : public ClientBase {
public:
    Client(std::unique_ptr<AuthBase> auth) : m_auth(std::move(auth)) {}
    ...
private:
    std::unique_ptr<AuthBase> m_auth;
};

class ClientAPI : public Client {
public:
    ClientAPI(const std::string &api_key, const std::string &api_secret) : 
        Client(std::make_unique<APIAuthetication>(api_key, api_secret)) {}
    ...
};

Thank you for the answer. I already try to changed the Client constructor to not take a raw pointer and also made the destructor virtual. I've only missed the override so it was calling AuthBase destructor but not the derivated.

Collectives™ on Stack Overflow

Memory leak with std::string depends on string size

1 Answer 1

1 Comment

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Related