Merge #18551: Do not clear validationinterface entries being executed
2276339a17
Add test for UnregisterAllValidationInterfaces bug (Russell Yanofsky)3c61abbbc8
Do not clear validationinterface entries being executed (Pieter Wuille) Pull request description: The previous code for MainSignalsInstance::Clear would decrement the reference count of every interface, including ones that were already Unregister()ed but still being executed. This fixes the issue pointed out here: https://github.com/bitcoin/bitcoin/pull/18524/files#r404395685 . It's not currently observable. ACKs for top commit: jonasschnelli: utACK2276339a17
- reviewed code and test (thanks @ryanofsky for adding the test). MarcoFalke: ACK2276339a17
🎎 ryanofsky: Code review ACK2276339a17
. No change to bugfix, just rebased and new test commit added since last review Tree-SHA512: c1d68e7c681a45c6cadc84e407c2266bcb4b12d34264e1232a61c4eadb74b551231c5a3b1d041de39f507aef4dfa7d4589b8bfe1833f069c739c6270d2a05dbe
This commit is contained in:
commit
1f70185a80
3 changed files with 66 additions and 2 deletions
|
@ -239,6 +239,7 @@ BITCOIN_TESTS =\
|
|||
test/util_tests.cpp \
|
||||
test/validation_block_tests.cpp \
|
||||
test/validation_flush_tests.cpp \
|
||||
test/validationinterface_tests.cpp \
|
||||
test/versionbits_tests.cpp
|
||||
|
||||
if ENABLE_WALLET
|
||||
|
|
63
src/test/validationinterface_tests.cpp
Normal file
63
src/test/validationinterface_tests.cpp
Normal file
|
@ -0,0 +1,63 @@
|
|||
// Copyright (c) 2020 The Bitcoin Core developers
|
||||
// Distributed under the MIT software license, see the accompanying
|
||||
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
|
||||
|
||||
#include <boost/test/unit_test.hpp>
|
||||
|
||||
#include <consensus/validation.h>
|
||||
#include <primitives/block.h>
|
||||
#include <scheduler.h>
|
||||
#include <util/check.h>
|
||||
#include <validationinterface.h>
|
||||
|
||||
BOOST_AUTO_TEST_SUITE(validationinterface_tests)
|
||||
|
||||
class TestInterface : public CValidationInterface
|
||||
{
|
||||
public:
|
||||
TestInterface(std::function<void()> on_call = nullptr, std::function<void()> on_destroy = nullptr)
|
||||
: m_on_call(std::move(on_call)), m_on_destroy(std::move(on_destroy))
|
||||
{
|
||||
}
|
||||
virtual ~TestInterface()
|
||||
{
|
||||
if (m_on_destroy) m_on_destroy();
|
||||
}
|
||||
void BlockChecked(const CBlock& block, const BlockValidationState& state) override
|
||||
{
|
||||
if (m_on_call) m_on_call();
|
||||
}
|
||||
static void Call()
|
||||
{
|
||||
CBlock block;
|
||||
BlockValidationState state;
|
||||
GetMainSignals().BlockChecked(block, state);
|
||||
}
|
||||
std::function<void()> m_on_call;
|
||||
std::function<void()> m_on_destroy;
|
||||
};
|
||||
|
||||
// Regression test to ensure UnregisterAllValidationInterfaces calls don't
|
||||
// destroy a validation interface while it is being called. Bug:
|
||||
// https://github.com/bitcoin/bitcoin/pull/18551
|
||||
BOOST_AUTO_TEST_CASE(unregister_all_during_call)
|
||||
{
|
||||
bool destroyed = false;
|
||||
|
||||
CScheduler scheduler;
|
||||
GetMainSignals().RegisterBackgroundSignalScheduler(scheduler);
|
||||
RegisterSharedValidationInterface(std::make_shared<TestInterface>(
|
||||
[&] {
|
||||
// First call should decrements reference count 2 -> 1
|
||||
UnregisterAllValidationInterfaces();
|
||||
BOOST_CHECK(!destroyed);
|
||||
// Second call should not decrement reference count 1 -> 0
|
||||
UnregisterAllValidationInterfaces();
|
||||
BOOST_CHECK(!destroyed);
|
||||
},
|
||||
[&] { destroyed = true; }));
|
||||
TestInterface::Call();
|
||||
BOOST_CHECK(destroyed);
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_SUITE_END()
|
|
@ -67,8 +67,8 @@ public:
|
|||
void Clear()
|
||||
{
|
||||
LOCK(m_mutex);
|
||||
for (auto it = m_list.begin(); it != m_list.end();) {
|
||||
it = --it->count ? std::next(it) : m_list.erase(it);
|
||||
for (const auto& entry : m_map) {
|
||||
if (!--entry.second->count) m_list.erase(entry.second);
|
||||
}
|
||||
m_map.clear();
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue