Differential D2918
test: Plug memory leaks and stack-use-after-scope Authored by Fabien on May 3 2019, 13:16.
Details
Backport of core PR12477 The modifications in checkqueue_tests.cpp are already part of our The modifications of test_bitcoin are only required to clear memory Depends on D2919 ./src/bench/bitcoin-bench
Diff Detail
Event TimelineComment Actions Please still include the change in test_bitcoin.cpp. Although it appears useless, there's no telling if Core will write a new test in the future that depends on this, which we would then want to backport. Essentially, by leaving this change out, we are taking ownership of the code. Unless we also want to own net.h's definitions and switch vNodes to a vector of smart pointers, then I don't think this half-ownership is wise. Comment Actions The issue is that the modification in test_bitcoin is incompatible with avalanche_tests implementation and DoS_tests since D2919 (it leads to double free, crashing the tests). These facilities were (before avalanche) specific to DoS_tests and are moved to the test file in https://github.com/bitcoin/bitcoin/commit/136bd7926c72659dd277a7b795ea17f72e523338#diff-d5ba361c5f8be78eb4cc0c787c1fc78e. Because I found the solution of the unique_ptr more elegant I preferred to use this to update DoS_tests rather than removing it from avalanche_tests which I think would reduce the quality of the code. Please review again with this input and share your thought. Comment Actions @deadalnix As mentioned in the summary, it has already been ported. You added the test in D1771 with the fix included. |