9 files changed, 75 insertions, 14 deletions
diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index fa0fbca8e8..e775ae43a1 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -243,6 +243,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
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index c132f265d2..f71ed99652 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -2347,6 +2347,8 @@ UniValue dumptxoutset(const JSONRPCRequest& request)
     return result;
 }
 
+void RegisterBlockchainRPCCommands(CRPCTable &t)
+{
 // clang-format off
 static const CRPCCommand commands[] =
 { //  category              name                      actor (function)         argNames
@@ -2387,8 +2389,6 @@ static const CRPCCommand commands[] =
 };
 // clang-format on
 
-void RegisterBlockchainRPCCommands(CRPCTable &t)
-{
     for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++)
         t.appendCommand(commands[vcidx].name, &commands[vcidx]);
 }
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index bde19d8e79..da9d583fa7 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -1022,6 +1022,8 @@ static UniValue estimaterawfee(const JSONRPCRequest& request)
     return result;
 }
 
+void RegisterMiningRPCCommands(CRPCTable &t)
+{
 // clang-format off
 static const CRPCCommand commands[] =
 { //  category              name                      actor (function)         argNames
@@ -1043,8 +1045,6 @@ static const CRPCCommand commands[] =
 };
 // clang-format on
 
-void RegisterMiningRPCCommands(CRPCTable &t)
-{
     for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++)
         t.appendCommand(commands[vcidx].name, &commands[vcidx]);
 }
diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp
index c87c1a5418..51a9581349 100644
--- a/src/rpc/misc.cpp
+++ b/src/rpc/misc.cpp
@@ -589,6 +589,8 @@ static UniValue echo(const JSONRPCRequest& request)
     return request.params;
 }
 
+void RegisterMiscRPCCommands(CRPCTable &t)
+{
 // clang-format off
 static const CRPCCommand commands[] =
 { //  category              name                      actor (function)         argNames
@@ -610,8 +612,6 @@ static const CRPCCommand commands[] =
 };
 // clang-format on
 
-void RegisterMiscRPCCommands(CRPCTable &t)
-{
     for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++)
         t.appendCommand(commands[vcidx].name, &commands[vcidx]);
 }
diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
index caa62ca958..10562126db 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -760,6 +760,8 @@ static UniValue getnodeaddresses(const JSONRPCRequest& request)
     return ret;
 }
 
+void RegisterNetRPCCommands(CRPCTable &t)
+{
 // clang-format off
 static const CRPCCommand commands[] =
 { //  category              name                      actor (function)         argNames
@@ -780,8 +782,6 @@ static const CRPCCommand commands[] =
 };
 // clang-format on
 
-void RegisterNetRPCCommands(CRPCTable &t)
-{
     for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++)
         t.appendCommand(commands[vcidx].name, &commands[vcidx]);
 }
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index ae3f15cec2..3c0b060439 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -1813,6 +1813,8 @@ UniValue analyzepsbt(const JSONRPCRequest& request)
     return result;
 }
 
+void RegisterRawTransactionRPCCommands(CRPCTable &t)
+{
 // clang-format off
 static const CRPCCommand commands[] =
 { //  category              name                            actor (function)            argNames
@@ -1839,8 +1841,6 @@ static const CRPCCommand commands[] =
 };
 // clang-format on
 
-void RegisterRawTransactionRPCCommands(CRPCTable &t)
-{
     for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++)
         t.appendCommand(commands[vcidx].name, &commands[vcidx]);
 }
diff --git a/src/test/validationinterface_tests.cpp b/src/test/validationinterface_tests.cpp
new file mode 100644
index 0000000000..208be92852
--- /dev/null
+++ b/src/test/validationinterface_tests.cpp
@@ -0,0 +1,60 @@
+// 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 <test/util/setup_common.h>
+#include <util/check.h>
+#include <validationinterface.h>
+
+BOOST_FIXTURE_TEST_SUITE(validationinterface_tests, TestingSetup)
+
+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;
+    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()
diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
index c06647cb0d..11000774c0 100644
--- a/src/validationinterface.cpp
+++ b/src/validationinterface.cpp
@@ -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();
     }
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 769a343365..61ad2f1198 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -4256,6 +4256,8 @@ UniValue importprunedfunds(const JSONRPCRequest& request);
 UniValue removeprunedfunds(const JSONRPCRequest& request);
 UniValue importmulti(const JSONRPCRequest& request);
 
+void RegisterWalletRPCCommands(interfaces::Chain& chain, std::vector<std::unique_ptr<interfaces::Handler>>& handlers)
+{
 // clang-format off
 static const CRPCCommand commands[] =
 { //  category              name                                actor (function)                argNames
@@ -4319,8 +4321,6 @@ static const CRPCCommand commands[] =
 };
 // clang-format on
 
-void RegisterWalletRPCCommands(interfaces::Chain& chain, std::vector<std::unique_ptr<interfaces::Handler>>& handlers)
-{
     for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++)
         handlers.emplace_back(chain.handleRpc(commands[vcidx]));
 }