sentry_close not releasing memory when initialising options via std::string c_str

[gdeankrotos]

Sentry 0.7.10, MSVC 14.41.34120

I have been encountering leaks reported by the CRT debugger which trace back to sentry, specifically initialisation and shutdown.

Take the following test program:

int main()
{
    const std::string dsn = "<redacted>";
    const std::string dbPath = "C:\\Users\\George Dean\\source\\repos\\SentryTest\\out\\build\\x64-debug\\SentryTest\\.sentry-native";
    const std::string handlerPath = "C:\\Users\\George Dean\\source\\repos\\SentryTest\\sentry-native\\build\\crashpad_build\\handler\\Debug\\crashpad_handler.exe";
    const std::string appVersion = "0.0.1";
    const std::string environment = "develop";
    const std::string distributionIdentifier = "abcdef";

    _CrtMemState s1, s2, s3;
    _CrtMemCheckpoint(&s1);

    sentry_options_t* options = sentry_options_new();
    sentry_options_set_dsn(options, dsn.c_str());
    sentry_options_set_database_path(options, dbPath.c_str());
    sentry_options_set_handler_path(options, handlerPath.c_str());
    sentry_options_set_release(options, appVersion.c_str());
    sentry_options_set_environment(options, environment.c_str());
    sentry_options_set_dist(options, distributionIdentifier.c_str());
    sentry_options_set_debug(options, 1);
    sentry_init(options);

    sentry_close();

    _CrtMemCheckpoint(&s2);

    if (_CrtMemDifference(&s3, &s1, &s2))
    {
        _CrtMemDumpStatistics(&s3);
    }

    _CrtDumpMemoryLeaks();
    return 0;
}

The output is as follows:

15:32:56:286    0 bytes in 0 Free Blocks.
15:32:56:286    48 bytes in 1 Normal Blocks.
15:32:56:286    29442 bytes in 110 CRT Blocks.
15:32:56:286    0 bytes in 0 Ignore Blocks.
15:32:56:286    0 bytes in 0 Client Blocks.
15:32:56:286    Largest number used: 40841 bytes.
15:32:56:286    Total allocations: 120943 bytes.
15:32:56:286    Detected memory leaks!
15:32:56:286    Dumping objects ->
15:32:56:286    {442} normal block at 0x000002547EC4D360, 48 bytes long.
15:32:56:286     Data: <                > 00 00 00 00 CD CD CD CD FF FF FF FF FF FF FF FF 
15:32:56:286    {191} normal block at 0x000002547EC45A70, 16 bytes long.
15:32:56:286     Data: <    `           > F8 F7 BE 92 60 00 00 00 00 00 00 00 00 00 00 00 
15:32:56:286    {190} normal block at 0x000002547EC46010, 16 bytes long.
15:32:56:286     Data: <    `           > B8 F7 BE 92 60 00 00 00 00 00 00 00 00 00 00 00 
15:32:56:286    {189} normal block at 0x000002547EC46150, 16 bytes long.
15:32:56:286     Data: <x   `           > 78 F7 BE 92 60 00 00 00 00 00 00 00 00 00 00 00 
15:32:56:286    {188} normal block at 0x000002547EC28BB0, 128 bytes long.
15:32:56:286     Data: <C:\Users\George > 43 3A 5C 55 73 65 72 73 5C 47 65 6F 72 67 65 20 
15:32:56:286    {187} normal block at 0x000002547EC3A8F0, 16 bytes long.
15:32:56:286     Data: <8   `           > 38 F7 BE 92 60 00 00 00 00 00 00 00 00 00 00 00 
15:32:56:286    {186} normal block at 0x000002547EC2B360, 96 bytes long.
15:32:56:286     Data: <C:\Users\George > 43 3A 5C 55 73 65 72 73 5C 47 65 6F 72 67 65 20 
15:32:56:286    {185} normal block at 0x000002547EC3A760, 16 bytes long.
15:32:56:286     Data: <    `           > F8 F6 BE 92 60 00 00 00 00 00 00 00 00 00 00 00 
15:32:56:286    {184} normal block at 0x000002547EC2E620, 96 bytes long.
15:32:56:286     Data: <<redacted>> 68 74 74 70 73 3A 2F 2F 35 33 63 63 31 36 32 64 
15:32:56:286    {183} normal block at 0x000002547EC3A710, 16 bytes long.
15:32:56:286     Data: <    `           > B8 F6 BE 92 60 00 00 00 00 00 00 00 00 00 00 00 
15:32:56:286    Object dump complete.

I'm interested if std::string c_str is supported here. In my application we use a library with it's own String object, so we use std::string as an intermediary to get to c_str.

The allocation stack trace is as follows:

>   SentryTest.exe!operator new(unsigned __int64 size) Line 36  C++
    SentryTest.exe!std::_Default_allocate_traits::_Allocate(const unsigned __int64 _Bytes) Line 102 C++
    SentryTest.exe!std::_Allocate<16,std::_Default_allocate_traits>(const unsigned __int64 _Bytes) Line 227 C++
    SentryTest.exe!std::allocator<std::_Container_proxy>::allocate(const unsigned __int64 _Count) Line 955  C++
    SentryTest.exe!std::_Container_proxy_ptr12<std::allocator<std::_Container_proxy>>::_Container_proxy_ptr12<std::allocator<std::_Container_proxy>>(std::allocator<std::_Container_proxy> & _Al_, std::_Container_base12 & _Mycont) Line 1447  C++
    SentryTest.exe!std::string::_Construct<1,char const *>(const char * const _Arg, const unsigned __int64 _Count) Line 868 C++
    SentryTest.exe!std::string::basic_string<char,std::char_traits<char>,std::allocator<char>>(const char * const _Ptr) Line 750    C++
    SentryTest.exe!main() Line 22   C++

Replacing const std::string with char* inline, the string leaks go away, but a leak is still being detected from crashpad_backend_startup, from the following allocation:

    SentryTest.exe!operator new(unsigned __int64 size) Line 36  C++
>   SentryTest.exe!crashpad::`anonymous namespace'::CommonInProcessInitialization() Line 620    C++
    SentryTest.exe!crashpad::CrashpadClient::StartHandler(const base::FilePath & handler, const base::FilePath & database, const base::FilePath & metrics_dir, const std::string & url, const std::string & http_proxy, const std::map<std::string,std::string,std::less<std::string>,std::allocator<std::pair<std::string const ,std::string>>> & annotations, const std::vector<std::string,std::allocator<std::string>> & arguments, bool restartable, bool asynchronous_start, const std::vector<base::FilePath,std::allocator<base::FilePath>> & attachments) Line 660 C++
    SentryTest.exe!crashpad_backend_startup(sentry_backend_s * backend, const sentry_options_s * options) Line 442  C++
    SentryTest.exe!sentry_init(sentry_options_s * options) Line 153 C
    SentryTest.exe!main() Line 23   C++

Any insight into this would be much appreciated!

[supervacuus]

Hi @gdeankrotos!

I'm interested if std::string c_str is supported here. In my application we use a library with it's own String object, so we use std::string as an intermediary to get to c_str.

The allocation stack trace is as follows:

> SentryTest.exe!operator new(unsigned __int64 size) Line 36  C++
  SentryTest.exe!std::_Default_allocate_traits::_Allocate(const unsigned __int64 _Bytes) Line 102 C++
  SentryTest.exe!std::_Allocate<16,std::_Default_allocate_traits>(const unsigned __int64 _Bytes) Line 227 C++
  SentryTest.exe!std::allocator<std::_Container_proxy>::allocate(const unsigned __int64 _Count) Line 955  C++
  SentryTest.exe!std::_Container_proxy_ptr12<std::allocator<std::_Container_proxy>>::_Container_proxy_ptr12<std::allocator<std::_Container_proxy>>(std::allocator<std::_Container_proxy> & _Al_, std::_Container_base12 & _Mycont) Line 1447  C++
  SentryTest.exe!std::string::_Construct<1,char const *>(const char * const _Arg, const unsigned __int64 _Count) Line 868 C++
  SentryTest.exe!std::string::basic_string<char,std::char_traits<char>,std::allocator<char>>(const char * const _Ptr) Line 750    C++
  SentryTest.exe!main() Line 22   C++

Yes, the usage of std::string.c_str() is supported since it only returns a pointer to a NULL-terminated C string. However, we never take ownership of any strings (because, like in this example, you wouldn't want us to rely on the lifetime of client strings or even free them before).

So what happens here is that when we get your pointer to the string, we immediately clone the entire string it points to before assigning it to our internal structs. sentry_close() will then free the internal clone, leaving your std::string untouched. The stack trace shows a new initiated in your main, seemingly still allocated (not surprising, given RAII) but not owned by the Native SDK.

Replacing const std::string with char* inline, the string leaks go away, but a leak is still being detected from crashpad_backend_startup, from the following allocation:

  SentryTest.exe!operator new(unsigned __int64 size) Line 36  C++
> SentryTest.exe!crashpad::`anonymous namespace'::CommonInProcessInitialization() Line 620    C++
  SentryTest.exe!crashpad::CrashpadClient::StartHandler(const base::FilePath & handler, const base::FilePath & database, const base::FilePath & metrics_dir, const std::string & url, const std::string & http_proxy, const std::map<std::string,std::string,std::less<std::string>,std::allocator<std::pair<std::string const ,std::string>>> & annotations, const std::vector<std::string,std::allocator<std::string>> & arguments, bool restartable, bool asynchronous_start, const std::vector<base::FilePath,std::allocator<base::FilePath>> & attachments) Line 660 C++
  SentryTest.exe!crashpad_backend_startup(sentry_backend_s * backend, const sentry_options_s * options) Line 442  C++
  SentryTest.exe!sentry_init(sentry_options_s * options) Line 153 C
  SentryTest.exe!main() Line 23   C++

Conversely, this is an actual leak because the crashpad client doesn't provide any mechanism to deallocate its client state. Once we initialize the crashpad backend, the client will stay allocated until the process exits. Given that its primary usage is the detection of unwanted process exits, this is a fair trade-off. This also means the crashpad crash handler remains active beyond sentry_close() (on macOS and Windows), meaning that if your application crashes after sentry_close(), the crash will still be reported.

[gdeankrotos]

Thank you for the quick response @supervacuus, I'm glad to hear that the residing leak is expected there.

I spent a bit more time investigating the string issue and I think I've got to the bottom of it.

In my application, the leaking memory was always a file path. I noticed that sentry__path_from_str_n determines the size of the destination using sizeof(wchar_t), but wchar_t is different between Windows and unix platforms. Long story short, I found the method sentry_options_set_handler_pathw already exists to solve this exact problem.

I'm now just passing a wchar* directly to the source string, skipping std::string altogether. This has fixed the leak.

Thanks again!