diff options
author | fanquake <fanquake@gmail.com> | 2022-07-29 15:43:21 +0100 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2022-07-29 15:47:23 +0100 |
commit | 5871b5b5ab57a0caf9b7514eb162c491c83281d5 (patch) | |
tree | b2ca982303e301ae389a0bef181432b9377e5500 /src/test/fuzz | |
parent | b1c8ea45c9e0b658c86ed0bbbf6e0fe44cd40f1c (diff) | |
parent | dd065dae9fcebd6806ff67703ffa8128e80b97cc (diff) | |
download | bitcoin-5871b5b5ab57a0caf9b7514eb162c491c83281d5.tar.xz |
Merge bitcoin/bitcoin#25571: refactor: Make mapBlocksUnknownParent local, and rename it
dd065dae9fcebd6806ff67703ffa8128e80b97cc refactor: Make mapBlocksUnknownParent local, and rename it (Hennadii Stepanov)
Pull request description:
This PR is a second attempt at #19594. This PR has two motivations:
- Improve code hygiene by eliminating a global variable, `mapBlocksUnknownParent`
- Fix fuzz test OOM when running too long ([see #19594 comment](https://github.com/bitcoin/bitcoin/pull/19594#issuecomment-958801638))
A minor added advantage is to release `mapBlocksUnknownParent` memory when the reindexing phase is done. The current situation is somewhat similar to a memory leak because this map exists unused for the remaining lifetime of the process. It's true that this map should be empty of data elements after use, but its internal metadata (indexing structures, etc.) can have non-trivial size because there can be many thousands of simultaneous elements in this map.
This PR helps our efforts to reduce the use of global variables. This variable isn't just global, it's hidden inside a function (it looks like a local variable but has the `static` attribute).
This global variable exists because the `-reindex` processing code calls `LoadExternalBlockFile()` multiple times (once for each block file), but that function must preserve some state between calls (the `mapBlocksUnknownParent` map). This PR fixes this by allocating this map as a local variable in the caller's scope and passing it in on each call. When reindexing completes, the map goes out of scope and is deallocated.
I tested this manually by reindexing on mainnet and signet. Also, the existing `feature_reindex.py` functional test passes.
ACKs for top commit:
mzumsande:
re-ACK dd065dae9fcebd6806ff67703ffa8128e80b97cc
theStack:
re-ACK dd065dae9fcebd6806ff67703ffa8128e80b97cc
shaavan:
reACK dd065dae9fcebd6806ff67703ffa8128e80b97cc
Tree-SHA512: 9cd20e44d2fa1096dd405bc107bc065ea8f904f5b3f63080341b08d8cf57b790df565f58815c2f331377d044d5306708b4bf6bdfc5ef8d0ed85d8e97d744732c
Diffstat (limited to 'src/test/fuzz')
-rw-r--r-- | src/test/fuzz/load_external_block_file.cpp | 11 |
1 files changed, 9 insertions, 2 deletions
diff --git a/src/test/fuzz/load_external_block_file.cpp b/src/test/fuzz/load_external_block_file.cpp index bfa977520b..f4b7dc08fd 100644 --- a/src/test/fuzz/load_external_block_file.cpp +++ b/src/test/fuzz/load_external_block_file.cpp @@ -31,6 +31,13 @@ FUZZ_TARGET_INIT(load_external_block_file, initialize_load_external_block_file) if (fuzzed_block_file == nullptr) { return; } - FlatFilePos flat_file_pos; - g_setup->m_node.chainman->ActiveChainstate().LoadExternalBlockFile(fuzzed_block_file, fuzzed_data_provider.ConsumeBool() ? &flat_file_pos : nullptr); + if (fuzzed_data_provider.ConsumeBool()) { + // Corresponds to the -reindex case (track orphan blocks across files). + FlatFilePos flat_file_pos; + std::multimap<uint256, FlatFilePos> blocks_with_unknown_parent; + g_setup->m_node.chainman->ActiveChainstate().LoadExternalBlockFile(fuzzed_block_file, &flat_file_pos, &blocks_with_unknown_parent); + } else { + // Corresponds to the -loadblock= case (orphan blocks aren't tracked across files). + g_setup->m_node.chainman->ActiveChainstate().LoadExternalBlockFile(fuzzed_block_file); + } } |