Page MenuHomePhabricator
Files F13711480

D9105.id27422.diff
No OneTemporary
Actions

D9105.id27422.diff
View Options

diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -1000,7 +1000,7 @@
return result;
}
-class submitblock_StateCatcher : public CValidationInterface {
+class submitblock_StateCatcher final : public CValidationInterface {
public:
uint256 hash;
bool found;
@@ -1069,27 +1069,22 @@
}
bool new_block;
- submitblock_StateCatcher sc(block.GetHash());
- RegisterValidationInterface(&sc);
+ auto sc = std::make_shared<submitblock_StateCatcher>(block.GetHash());
+ RegisterSharedValidationInterface(sc);
bool accepted =
EnsureChainman(request.context)
.ProcessNewBlock(config, blockptr, /* fForceProcessing */ true,
/* fNewBlock */ &new_block);
- // We are only interested in BlockChecked which will have been dispatched
- // in-thread, so no need to sync before unregistering.
- UnregisterValidationInterface(&sc);
- // Sync to ensure that the catcher's slots aren't executing when it goes out
- // of scope and is deleted.
- SyncWithValidationInterfaceQueue();
+ UnregisterSharedValidationInterface(sc);
if (!new_block && accepted) {
return "duplicate";
}
- if (!sc.found) {
+ if (!sc->found) {
return "inconclusive";
}
- return BIP22ValidationResult(config, sc.state);
+ return BIP22ValidationResult(config, sc->state);
}
static UniValue submitheader(const Config &config,
diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp
--- a/src/test/validation_block_tests.cpp
+++ b/src/test/validation_block_tests.cpp
@@ -39,7 +39,7 @@
BOOST_FIXTURE_TEST_SUITE(validation_block_tests, MinerTestingSetup)
-struct TestSubscriber : public CValidationInterface {
+struct TestSubscriber final : public CValidationInterface {
uint256 m_expected_tip;
explicit TestSubscriber(uint256 tip) : m_expected_tip(tip) {}
@@ -201,8 +201,8 @@
LOCK(cs_main);
initial_tip = ::ChainActive().Tip();
}
- TestSubscriber sub(initial_tip->GetBlockHash());
- RegisterValidationInterface(&sub);
+ auto sub = std::make_shared<TestSubscriber>(initial_tip->GetBlockHash());
+ RegisterSharedValidationInterface(sub);
// create a bunch of threads that repeatedly process a block generated above
// at random this will create parallelism and randomness inside validation -
@@ -237,10 +237,10 @@
}
SyncWithValidationInterfaceQueue();
- UnregisterValidationInterface(&sub);
+ UnregisterSharedValidationInterface(sub);
LOCK(cs_main);
- BOOST_CHECK_EQUAL(sub.m_expected_tip,
+ BOOST_CHECK_EQUAL(sub->m_expected_tip,
::ChainActive().Tip()->GetBlockHash());
}
diff --git a/src/test/validationinterface_tests.cpp b/src/test/validationinterface_tests.cpp
--- a/src/test/validationinterface_tests.cpp
+++ b/src/test/validationinterface_tests.cpp
@@ -12,6 +12,39 @@
BOOST_FIXTURE_TEST_SUITE(validationinterface_tests, TestingSetup)
+struct TestSubscriberNoop final : public CValidationInterface {
+ void BlockChecked(const CBlock &, const BlockValidationState &) override {}
+};
+
+BOOST_AUTO_TEST_CASE(unregister_validation_interface_race) {
+ std::atomic<bool> generate{true};
+
+ // Start thread to generate notifications
+ std::thread gen{[&] {
+ const CBlock block_dummy;
+ const BlockValidationState state_dummy;
+ while (generate) {
+ GetMainSignals().BlockChecked(block_dummy, state_dummy);
+ }
+ }};
+
+ // Start thread to consume notifications
+ std::thread sub{[&] {
+ // keep going for about 1 sec, which is 250k iterations
+ for (int i = 0; i < 250000; i++) {
+ auto subscriber = std::make_shared<TestSubscriberNoop>();
+ RegisterSharedValidationInterface(subscriber);
+ UnregisterSharedValidationInterface(subscriber);
+ }
+ // tell the other thread we are done
+ generate = false;
+ }};
+
+ gen.join();
+ sub.join();
+ BOOST_CHECK(!generate);
+}
+
class TestInterface : public CValidationInterface {
public:
TestInterface(std::function<void()> on_call = nullptr,

File Metadata

Mime Type
text/plain
Expires
Sat, Apr 26, 12:11 (2 h, 51 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5573524
Default Alt Text
D9105.id27422.diff (4 KB)

Event Timeline