frang75/nappgui_src

nappgui may falsely report memory leaks at exit when linking with static libraries on windows

Closed this issue · 5 comments

colugomusic commented

In bmem_win.c, _bmem_atexit() is currently defined as:

void _bmem_atexit(void)
{
#if defined(__MEMORY_AUDITOR__)
#if _WITH_CRTDBG
    _CrtDumpMemoryLeaks();
#endif
#else
    cassert(i_HEAP == NULL);
/*HeapDestroy(i_HEAP);*/
#endif
}

I have found that this will falsely report memory leaks at exit if your application is linking with a library which still has static objects hanging around at the point where nappgui emits the call to _CrtDumpMemoryLeaks. In my case it is the Boost.process library. I think the order in which static objects across different linked libraries are deallocated is not well defined so nappgui should probably remove this call.

frang75 commented

Hi @colugomusic, thanks for using NAppGUI.

Indeed, with external libraries with static objects it is possible that _CrtDumpMemoryLeaks() detects memory leaks due to objects not yet released. However, this call is very useful during development, since native Win32 objects are not controlled by the NAppGUI memory auditor. At the moment, it is not going to be removed.

Control over the CRT (#include <crtdbg.h>) is only included in DEBUG builds. You can also disable it by commenting the line:

#define _WITH_CRTDBG 1

colugomusic commented

Hi @colugomusic, thanks for using NAppGUI.

Indeed, with external libraries with static objects it is possible that _CrtDumpMemoryLeaks() detects memory leaks due to objects not yet released. However, this call is very useful during development, since native Win32 objects are not controlled by the NAppGUI memory auditor. At the moment, it is not going to be removed.

Control over the CRT (#include <crtdbg.h>) is only included in DEBUG builds. You can also disable it by commenting the line:

#define _WITH_CRTDBG 1

Maybe a compromise could be to allow the user to opt out by passing an additional compiler flag, e.g. _NAPPGUI_NO_CRTDBG if they know they are linking against other libraries:

#if defined(_MSC_VER) && _MSC_VER > 1400 && !defined(_NAPPGUI_NO_CRTDBG)
#define _WITH_CRTDBG 1
#include <crtdbg.h>
#endif

That way they don't have to keep a separate fork of nappgui. Otherwise the output of _CrtDumpMemoryLeaks can add quite a lot of noise to the build output so it's a bit annoying to ignore.

  • 👍1
frang75 commented

There is a typo in doc. The correct option is -DCMAKE_DISABLE_CRTDBG=YES