search

dropbox / json11

A tiny JSON library for C++11.
MIT License
2.54k stars 613 forks source link

Remove invoking of the non-trivial destructor. #128

Open sabbakumov opened 5 years ago

sabbakumov commented 5 years ago

Don't expose the risk of accessing objects after the end of their lifetime

artwyman commented 5 years ago

I can see the purpose for this, and have worked in codebases where we would've made this tradeoff. Trading deinit-time safety for a resource leak feels like a trade-off which is situation-specific, though. E.g. in an app which needed to repeatedly load and unload a dynamic library this kind of memory leak can become meaningful, and even before that it can trigger warnings in people's toolchains. So, I don't think this is a bad idea, but I'm not sure about merging it into the standard json11 implementation.

sabbakumov commented 5 years ago

What do you think of the Chromium's NoDestructor class? It doesn't allocate new memory on library loading/unloading. But tools still can signal a memory leak warning. However, the memory blocks are still reachable, so I think it's a minor issue. This class is highly used inside Chromium. I've added this class to the Pull Request.