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

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

Closed
colugomusic opened this issue Sep 8, 2024 · 5 comments
Assignees
Labels
build-system Issue related with CMake or compilers

Comments

@colugomusic
Copy link

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
Copy link
Owner

frang75 commented Sep 9, 2024

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

@frang75 frang75 self-assigned this Sep 9, 2024
@colugomusic
Copy link
Author

colugomusic commented Sep 9, 2024

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.

@frang75 frang75 added the build-system Issue related with CMake or compilers label Sep 10, 2024
@frang75
Copy link
Owner

frang75 commented Sep 20, 2024

@colugomusic
Fixed in this commit: b65fc31
Doc: https://nappgui.com/docs/r5452/en/guide/build.html#h3

@colugomusic
Copy link
Author

@colugomusic Fixed in this commit: b65fc31 Doc: https://nappgui.com/docs/r5452/en/guide/build.html#h3

Perfect thank you!

@frang75
Copy link
Owner

frang75 commented Sep 20, 2024

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

@frang75 frang75 closed this as completed Sep 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-system Issue related with CMake or compilers
Projects
None yet
Development

No branches or pull requests

2 participants