Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/silence false positive memory leaks reported by Microsoft's debug CRT #1142

Merged
merged 4 commits into from
Aug 16, 2018

Conversation

scottslacksmith
Copy link
Contributor

There are few Issues (e.g. #624 , #692, #757 and #970) requesting a "memory leak" fix for MSVC debug builds. As previously noted by the comments of above issues, these "memory leaks" are false positives as they relate to memory allocations that are alive for the entire lifetime of the app. Irrespective, is still desirable to fix/silence these false positives leak to help identify real leaks when they arise during development.

The false positive memory leaks arise due to limitations of MSVC's lightweight memory leak detection mechanism. The MS debug CRT can only detect if a memory allocation is missing a matching deallocation. It can't determine, unlike valgrind, that the memory allocation is reachable for the entire lifetime of the app. Consequently the MS debug CRT will report a false positive leak for memory that’s intentionally never deallocated.

One possible solution to fix the false positive mem leaks is to ensure that every mem allocation has a matching deallocation even if this creates redundant mem deallocation work during app shutdown. However it could be difficult to get this solution correct for every compiler/platform combination. Instead it's probably better to get MS's debug CRT to temporarily suspend mem leak tracking for mem allocations to intentionally have no matching deallocation by using the following utility debug function
CrtSetDbgFlag(old_crtdbg_flag & ~_CRTDBG_ALLOC_MEM_DF);

The proposed patch adds a new RAII MemoryIsNotDeallocated class that temporarily suspend mem leak tracking. We use this RAII class to silence 2 false positive leaks that are caused by memory allocations that are intentionally never deallocated.

For example, the following are the false positive leaks reported before this change;

Detected memory leaks!
Dumping objects ->
{750} normal block at 0x015DF938, 8 bytes long.
Data: < ] > 00 F9 5D 01 00 00 00 00
{749} normal block at 0x015DEE60, 32 bytes long.
Data: <] ] ] > 60 EE 5D 01 60 EE 5D 01 60 EE 5D 01 01 01 CD CD {748} normal block at 0x015DF900, 12 bytes long. Data: <8 ] ] > 38 F9 5D 01 60 EE 5D 01 00 00 00 00
{747} normal block at 0x015DA0F8, 24 bytes long.
Data: < > FF FF FF FF FF FF FF FF 00 00 00 00 00 00 00 00
Object dump complete.

As you can see from above it’s not easy to identify the above are false positives. Consequently, if false positive leaks are not fixed or silenced, then it becomes impractical to identify real memory leaks.

Notes

The memory leak report can be enabled by calling the following function
#ifdef _MSC_VER
_CrtSetDbgFlag(_CrtSetDbgFlag(_CRTDBG_REPORT_FLAG) | _CRTDBG_LEAK_CHECK_DF);
#endif // _MSC_VER
anywhere before exiting main.

(The MS debug CRT is always tracking memory allocations but the final memory leak report is disabled by default. As you can’t avoid paying for its cost, you may as well use it!)

Add a new RAII MemoryIsNotDeallocated class that excludes memory allocations from Microsoft’s debug CRT leak detection report.
We use this RAII class to silence 2 false positive leaks that are caused by memory allocations that are intentionally never deallocated.

*Background*
The MS debug CRT has a lightweight memory leak detection mechanism that can only detect if a memory allocation is missing a matching deallocation.
Consequently, it will report a false positive leak for memory that’s intentionally never deallocated. For example, memory that’s reachable for the entire lifetime of a app.
Note the MS debug CRT is always tracking memory allocations but the final memory leak report is disabled by default. As you can’t avoid paying for its cost, you may as well use it.

The memory leak report can be enabled by calling the following function
  #ifdef _MSC_VER
  _CrtSetDbgFlag(_CrtSetDbgFlag(_CRTDBG_REPORT_FLAG) | _CRTDBG_LEAK_CHECK_DF);
  #endif // _MSC_VER
anywhere before exiting main.

For example, the following are the false positive leaks reported before this change;

Detected memory leaks!
Dumping objects ->
{750} normal block at 0x015DF938, 8 bytes long.
Data: <  ]     > 00 F9 5D 01 00 00 00 00
{749} normal block at 0x015DEE60, 32 bytes long.
Data: <` ] ` ] ` ]     > 60 EE 5D 01 60 EE 5D 01 60 EE 5D 01 01 01 CD CD
{748} normal block at 0x015DF900, 12 bytes long.
Data: <8 ] ` ]     > 38 F9 5D 01 60 EE 5D 01 00 00 00 00
{747} normal block at 0x015DA0F8, 24 bytes long.
Data: <                > FF FF FF FF FF FF FF FF 00 00 00 00 00 00 00 00
Object dump complete.

As you can see from above it’s not easy to identify the above are false positives. Consequently, if false positive leaks are not fixed or silenced, then it becomes impractical to identify real memory leaks.
@dasimx
Copy link

dasimx commented Apr 10, 2018

@scottslacksmith: Thanks so much! Your patch finally makes the VS leak detector usable again with gtest! :)

Just small style changes and we can accept this PR
@lyn444
Copy link

lyn444 commented Aug 22, 2018

thanks for the patch! been waiting for a long time.

@Jiwan
Copy link

Jiwan commented May 14, 2019

It seems that this fix the issue with MSVC leak detector, but not VLD

It makes it impossible to run tests and check that nothing leak on them on Windows. If it was a false-positive from VLD, it would not be an issue. But these are actual leaks that will trigger many more tools than just Visual Studio integrated ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants