LeakSanitizer detects memory leak if CDBWrapper ctor fails #22592

1. 
   Crypt-iQ commented at 5:44 pm on July 31, 2021: contributor

   While running the functional tests under LSAN and 8 parallel jobs, CDBWrapper ctor would regularly throw via HandleError because of low disk space. This would make LSAN error because the destructor for CDBWrapper was never called and so the options.block_cache and some other parameters wouldn’t be destructed.

   LSAN output (I deleted the file, but managed to take a picture of it, so log is kind of truncated):

    0==1204791==ERROR:  LeakSanitizer: detected memory leaks
    1
    2Direct leak of 3520 byte(s) in 1 object(s) allocated from:
    3       [#0](/bitcoin-bitcoin/0/) 0x55f632912679 in operator new(unsigned long)
    4       [#1](/bitcoin-bitcoin/1/) 0x55f6342936d6 in leveldb::NewLRUCache(unsigned long)
    5       [#2](/bitcoin-bitcoin/2/) 0x55f6335f66f8 in GetOptions(unsigned long)
    6       [#3](/bitcoin-bitcoin/3/) 0x55f6335f4ec7 in CDBWrapper::CDBWrapper(boost::filesystem::path const&, unsigned long, bool, bool, bool)
    7       [#4](/bitcoin-bitcoin/4/) 0x55f6332e32cf in std::_MakeUniq<CDBWrapper>::__single_object std::make_unique<CoinsViews ...truncated...
    8       [#5](/bitcoin-bitcoin/5/) 0x55f6332d2781 in CCoinsViewDB::CCoinsViewDB(boost::filesystem::path, unsigned long, bool, bool)
    9       [#6](/bitcoin-bitcoin/6/) 0x55f6333ec6e2 in CoinsViews::CoinsViews(std::__cxx11::basic_string<char, std::char_traits<char>, ...truncated...
   10       [#7](/bitcoin-bitcoin/7/) 0x55f63346a111 in std::_MakeUniq<CoinsViews>::__single_object std::make_unique<CoinsViews, ...truncated...
   11       [#8](/bitcoin-bitcoin/8/) 0x55f6333ed379 in CChainState::InitCoinsDB(unsigned long, bool, bool, std::__cxx11::basic_string ...truncated...
   12       [#9](/bitcoin-bitcoin/9/) 0x55f6329699e0 in AppInitMain(NodeContext&, interfaces::BlockAndHeaderTipInfo*)
   13       [#10](/bitcoin-bitcoin/10/) 0x55f63291868e in AppInit(NodeContext&, int, char**)
   14       [#11](/bitcoin-bitcoin/11/) 0x55f632915e2f in main
   15       [#12](/bitcoin-bitcoin/12/) 0x7f86b6f7fb24 in __libc_start_main
   

2. 
   MarcoFalke commented at 6:02 pm on July 31, 2021: member

   Is this a bug report for upstream leveldb or just a note or something else? If you want to suppress the warning you can add a suppression to suppressions file.
3. 
   Crypt-iQ commented at 6:10 pm on July 31, 2021: contributor

   I wasn’t sure what would be the preferred method to handle this especially since it’s an unlikely occurrence. For code correctness, it could check the status and call delete before HandleError, or a suppression could be added.
4. 
   MarcoFalke commented at 6:22 pm on July 31, 2021: member

   Wrapping it into a unique_ptr could also help?
5. 
   Crypt-iQ commented at 3:03 pm on August 2, 2021: contributor

   Yes I think wrapping options in a unique_ptr would work – I can submit a patch
6. 
   Crypt-iQ commented at 2:15 pm on August 4, 2021: contributor

   Actually I don’t think wrapping in a unique_ptr will help because it won’t free the leveldb::Options raw pointers. So working on a patch without unique_ptr.

Contributors
Crypt-iQ MarcoFalke