LeakSanitizer detects memory leak if CDBWrapper ctor fails #22592

issue Crypt-iQ openend this issue on July 31, 2021
  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. maflcko 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. maflcko 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.
  7. willcl-ark commented at 11:19 am on January 16, 2026: member

    @Crypt-iQ do you think this is likely still an issue? It seems to be difficult to hit (I can’t reproduce on a few tries locally), and the PR didn’t get enough time dedicated to it to see it through.

    Do you think it’s worth keeping open to see if someone else will pick it up?

  8. willcl-ark added the label Bug on Jan 16, 2026
  9. maflcko commented at 11:41 am on January 16, 2026: member

    It seems to be difficult to hit (I can’t reproduce on a few tries locally)

    I think IO fault injection could help here as well, ignoring any program state and program exit code, and only looking for sanitizer issues.

    I wanted to implement this for a long time. cc @dergoegge you may also want to try this in your infra?

  10. maflcko commented at 11:43 am on January 16, 2026: member
    (also removed the ‘up for grabs’ from the pull, as this issue is open, so it is clear that this may be up-for-grabs, as long as the issue is open)
  11. Crypt-iQ commented at 6:14 pm on January 16, 2026: contributor

    @Crypt-iQ do you think this is likely still an issue? It seems to be difficult to hit (I can’t reproduce on a few tries locally), and the PR didn’t get enough time dedicated to it to see it through.

    Do you think it’s worth keeping open to see if someone else will pick it up?

    I’m still able to reproduce by running under address sanitizer and ASAN_OPTIONS="detect_leaks=1" with the following diff on top of 0ffb20dee17858e0f13b23fce3dd0ec06cbeb9a2 (latest commit should work as well):

     0diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp
     1index b3f08cb20d..ebee13d120 100644
     2--- a/src/dbwrapper.cpp
     3+++ b/src/dbwrapper.cpp
     4@@ -240,7 +240,8 @@ CDBWrapper::CDBWrapper(const DBParams& params)
     5     // on Windows it converts from UTF-8 to UTF-16 before calling ::CreateFileW
     6     // (see env_posix.cc and env_windows.cc).
     7     leveldb::Status status = leveldb::DB::Open(DBContext().options, fs::PathToString(params.path), &DBContext().pdb);
     8-    HandleError(status);
     9+    const std::string errmsg = "Fatal!";
    10+    throw dbwrapper_error(errmsg);
    11     LogInfo("Opened LevelDB successfully");
    12 
    13     if (params.options.force_compact) {
    

    I think it’s pretty benign but it’s good to not have false positives so I think this should stay open?


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-01-16 21:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me