arximboldi / immer

Postmodern immutable and persistent data structures for C++ — value semantics at scale
https://sinusoid.es/immer
Boost Software License 1.0
2.51k stars 185 forks source link

Memory leak in debug builds #241

Closed richtw1 closed 1 year ago

richtw1 commented 1 year ago

VS2022 complaining about leaked 296 byte blocks in debug builds (i.e. NDEBUG not defined). Using the _CrtSetDbgFlag( _CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF ); diagnostics from the Visual Studio runtime, setting breakpoints on the allocation IDs flagged in the log, I was able to determine that the problematic allocations come from debug_size_heap::allocate() when making a new rb tree leaf. I haven't dug into the details, but the problem appears to be that, in free_list_heap::deallocate(), base_t::deallocate() is never getting called (because the condition above count >= Limit is always false).

No such problem with release builds, although it doesn't appear to get anywhere near this code either.

arximboldi commented 1 year ago

Not sure this is an actual leak. It feels just like this is considering the top level blocks that we keep around for reuse in the free_list as "leaks", even though they are technically not.

richtw1 commented 1 year ago

Is this something a global destructor or a immer::shutdown() type function can help with, just to silence the "alloc without dealloc" diagnostics?

Thanks so much for immer, I'm enjoying using it immensely for a small home project I'm working on. Next step is to figure out how to best implement unidirectional dataflow with wxWidgets, which wants you to work in a much more traditional way...

richtw1 commented 1 year ago

For further information, VS is reporting two "leaks" per immer::vector which has been constructed, rather than a fixed number. I don't know if that corroborates your idea about it being the top level blocks which are kept around or not (I would have expected them to be a more global thing).

arximboldi commented 1 year ago

This is very odd because LeakSanitizer does not report any leaks, and also the library has been thoroughly used in production environments. Does building with #define IMMER_NO_FREE_LIST 1 remove the leak warnings?

arximboldi commented 1 year ago

https://stackoverflow.com/questions/2323458/how-to-ignore-false-positive-memory-leaks-from-crtdumpmemoryleaks

arximboldi commented 1 year ago

I do believe this is really a false positive. Deallocate is not called because the buffer is kept in the global free list for later potential reuse. We could in the destructor of the free list go through the global free list and deallocate the objects, but that is not really necessary becuase this is just a constant size amount of raw memory (no side-effects) so it is not needed to deallocate it on process exit. I do really believe this is not really an issue.

arximboldi commented 1 year ago

Of course if you find a nice way to silence the false positve warning from Immer itself, I'm happy to integrate it.