search

backtrace-labs / crashpad

A fork of Crashpad with file attachment support and other improvements.
Apache License 2.0
99 stars 33 forks source link

Memory leak in crashpad/client/crashpad_client_win.cc #4

Closed dh0well closed 4 years ago

dh0well commented 4 years ago

Through the use of the Microsoft CRT memory leak detection mechanism, I was seeing a report in our application with this announcement:

Detected memory leaks!
Dumping objects ->
{3844} normal block at 0x000001F31FDC3970, 48 bytes long.
 Data: <                > 00 00 00 00 CD CD CD CD FF FF FF FF FF FF FF FF 
Object dump complete.

I tracked down the potential source of the leak and it would appear to be this object: https://github.com/backtrace-labs/crashpad/blob/6c9ef3e8b0cb2f7facd00ad9b786fc493139d16f/client/crashpad_client_win.cc#L554

void CommonInProcessInitialization()
{
   ...
  g_non_crash_dump_lock = new base::Lock(); // <---- implementation lacks a delete (use a smart pointer here?)
}
pkhuong commented 4 years ago

Unfortunately, it is not safe to use smart pointers (or other objects with non-trivial destructors) for globals; see, for example, https://isocpp.org/wiki/faq/ctors#construct-on-first-use-v2 for details. That's why Google's C++ style guide mandates the kind of controlled leaking https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables you observe in the crashpad project (which adheres to that style guide).

The only way this line can actually leak resources is if the calling program initialises crashpad multiple times in the same process, e.g., by calling CrashpadClient::StartHandler() or CrashpadClient::SetHandlerIPCPipe repeatedly. That would be a programmer error and there are bigger issues in such a situations than a mere resource leak.

GaryOrlando commented 2 years ago

"Controlled leaking" I've never heard of such a thing in my 30+ years of development. !

None of our released apps gets through testing with leaks identified by the compiler. I can't believe this is acceptable.. I guess you get what you pay for.