blockstorage: Avoid potential Memory Leak #30932

1. 
   LMAO798 commented at 8:16 pm on September 19, 2024: none

   Issue: -> Potential Memory Leak in BlockManager::InsertBlockIndex Reason: -> The InsertBlockIndex function creates a new CBlockIndex when try_emplace is successful, but doesn’t guarantee that the object will be released if an error occurs later. This can lead to memory leaks potentially.
2. blockstorage: Avoid potential Memory Leak d6f2235a1b
3. 
   DrahtBot commented at 8:16 pm on September 19, 2024: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

4. DrahtBot added the label Block storage on Sep 19, 2024
5. DrahtBot added the label CI failed on Sep 19, 2024
6. 
   DrahtBot commented at 9:34 pm on September 19, 2024: contributor

   🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/30398888961

   Make sure to run all tests locally, according to the documentation.

   The failure may happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

7. 
   jonatack commented at 10:05 pm on September 19, 2024: member

   @LMAO798 the validation_chainstatemanager_tests unit test file is failing with your patch. You should be able to reproduce the issue locally by building and then running:

   0./build/src/test/test_bitcoin --run_test=validation_chainstatemanager_tests
   

   Would the following work instead?

   0    const auto [mi, inserted]{m_block_index.try_emplace(hash)};
   1    if (!inserted) {
   2        return &mi->second;
   3    }
   4    CBlockIndex* pindex = &(*mi).second;
   5    pindex->phashBlock = &((*mi).first);
   6    return pindex;
   

8. 
   maflcko commented at 5:53 am on September 20, 2024: member

   Closing as a low-quality (and obviously wrong) LLM generated bot/spam patch, with an (obviously wrong) hallucinated explanation, without any test coverage or otherwise steps to test, and finally test failures in the existing tests.
9. maflcko closed this on Sep 20, 2024

   ---

Contributors
LMAO798 DrahtBot jonatack maflcko

Labels
Block storage CI failed