Merge #18524: refactor: drop boost::signals2 in validationinterface

d6815a2313158862d448733954a73520f223deb6 refactor: drop boost::signals2 in validationinterface (Russell Yanofsky)

Pull request description:

  Stop using boost::signals2 internally in validationinterface. Replace with std::list and Add/Remove/Clear/Iterate helper functions.

  Motivation for change is to reduce dependencies and avoid issues happening with boost versions before 1.59: https://github.com/bitcoin/bitcoin/issues/18517, https://github.com/bitcoin/bitcoin/pull/18471

ACKs for top commit:
  MarcoFalke:
    ACK d6815a2313158862d448733954a73520f223deb6
  laanwj:
    ACK d6815a2313158862d448733954a73520f223deb6
  hebasto:
    re-ACK d6815a2313158862d448733954a73520f223deb6
  promag:
    ACK d6815a2313158862d448733954a73520f223deb6.

Tree-SHA512: 4fc0f14a8446e8616cc142af6c3d36815f3254525d30348ba8e4d4bc74c249a5a8c9bc119bdd1be7ebd7abe0b784bc0c5551a3e156a766890cb2fdd891a95919

2 files changed, 74 insertions, 45 deletions

diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
index f9f61e8a09..c06647cb0d 100644
--- a/src/validationinterface.cpp
+++ b/src/validationinterface.cpp
@@ -16,36 +16,75 @@
 #include <unordered_map>
 #include <utility>
 
-#include <boost/signals2/signal.hpp>
-
-struct ValidationInterfaceConnections {
-    boost::signals2::scoped_connection UpdatedBlockTip;
-    boost::signals2::scoped_connection TransactionAddedToMempool;
-    boost::signals2::scoped_connection BlockConnected;
-    boost::signals2::scoped_connection BlockDisconnected;
-    boost::signals2::scoped_connection TransactionRemovedFromMempool;
-    boost::signals2::scoped_connection ChainStateFlushed;
-    boost::signals2::scoped_connection BlockChecked;
-    boost::signals2::scoped_connection NewPoWValidBlock;
-};
-
+//! The MainSignalsInstance manages a list of shared_ptr<CValidationInterface>
+//! callbacks.
+//!
+//! A std::unordered_map is used to track what callbacks are currently
+//! registered, and a std::list is to used to store the callbacks that are
+//! currently registered as well as any callbacks that are just unregistered
+//! and about to be deleted when they are done executing.
 struct MainSignalsInstance {
-    boost::signals2::signal<void (const CBlockIndex *, const CBlockIndex *, bool fInitialDownload)> UpdatedBlockTip;
-    boost::signals2::signal<void (const CTransactionRef &)> TransactionAddedToMempool;
-    boost::signals2::signal<void (const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex)> BlockConnected;
-    boost::signals2::signal<void (const std::shared_ptr<const CBlock>&, const CBlockIndex* pindex)> BlockDisconnected;
-    boost::signals2::signal<void (const CTransactionRef &)> TransactionRemovedFromMempool;
-    boost::signals2::signal<void (const CBlockLocator &)> ChainStateFlushed;
-    boost::signals2::signal<void (const CBlock&, const BlockValidationState&)> BlockChecked;
-    boost::signals2::signal<void (const CBlockIndex *, const std::shared_ptr<const CBlock>&)> NewPoWValidBlock;
-
+private:
+    Mutex m_mutex;
+    //! List entries consist of a callback pointer and reference count. The
+    //! count is equal to the number of current executions of that entry, plus 1
+    //! if it's registered. It cannot be 0 because that would imply it is
+    //! unregistered and also not being executed (so shouldn't exist).
+    struct ListEntry { std::shared_ptr<CValidationInterface> callbacks; int count = 1; };
+    std::list<ListEntry> m_list GUARDED_BY(m_mutex);
+    std::unordered_map<CValidationInterface*, std::list<ListEntry>::iterator> m_map GUARDED_BY(m_mutex);
+
+public:
     // We are not allowed to assume the scheduler only runs in one thread,
     // but must ensure all callbacks happen in-order, so we end up creating
     // our own queue here :(
     SingleThreadedSchedulerClient m_schedulerClient;
-    std::unordered_map<CValidationInterface*, ValidationInterfaceConnections> m_connMainSignals;
 
     explicit MainSignalsInstance(CScheduler *pscheduler) : m_schedulerClient(pscheduler) {}
+
+    void Register(std::shared_ptr<CValidationInterface> callbacks)
+    {
+        LOCK(m_mutex);
+        auto inserted = m_map.emplace(callbacks.get(), m_list.end());
+        if (inserted.second) inserted.first->second = m_list.emplace(m_list.end());
+        inserted.first->second->callbacks = std::move(callbacks);
+    }
+
+    void Unregister(CValidationInterface* callbacks)
+    {
+        LOCK(m_mutex);
+        auto it = m_map.find(callbacks);
+        if (it != m_map.end()) {
+            if (!--it->second->count) m_list.erase(it->second);
+            m_map.erase(it);
+        }
+    }
+
+    //! Clear unregisters every previously registered callback, erasing every
+    //! map entry. After this call, the list may still contain callbacks that
+    //! are currently executing, but it will be cleared when they are done
+    //! executing.
+    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);
+        }
+        m_map.clear();
+    }
+
+    template<typename F> void Iterate(F&& f)
+    {
+        WAIT_LOCK(m_mutex, lock);
+        for (auto it = m_list.begin(); it != m_list.end();) {
+            ++it->count;
+            {
+                REVERSE_LOCK(lock);
+                f(*it->callbacks);
+            }
+            it = --it->count ? std::next(it) : m_list.erase(it);
+        }
+    }
 };
 
 static CMainSignals g_signals;
@@ -78,15 +117,7 @@ CMainSignals& GetMainSignals()
 void RegisterSharedValidationInterface(std::shared_ptr<CValidationInterface> pwalletIn) {
     // Each connection captures pwalletIn to ensure that each callback is
     // executed before pwalletIn is destroyed. For more details see #18338.
-    ValidationInterfaceConnections& conns = g_signals.m_internals->m_connMainSignals[pwalletIn.get()];
-    conns.UpdatedBlockTip = g_signals.m_internals->UpdatedBlockTip.connect(std::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3));
-    conns.TransactionAddedToMempool = g_signals.m_internals->TransactionAddedToMempool.connect(std::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, std::placeholders::_1));
-    conns.BlockConnected = g_signals.m_internals->BlockConnected.connect(std::bind(&CValidationInterface::BlockConnected, pwalletIn, std::placeholders::_1, std::placeholders::_2));
-    conns.BlockDisconnected = g_signals.m_internals->BlockDisconnected.connect(std::bind(&CValidationInterface::BlockDisconnected, pwalletIn, std::placeholders::_1, std::placeholders::_2));
-    conns.TransactionRemovedFromMempool = g_signals.m_internals->TransactionRemovedFromMempool.connect(std::bind(&CValidationInterface::TransactionRemovedFromMempool, pwalletIn, std::placeholders::_1));
-    conns.ChainStateFlushed = g_signals.m_internals->ChainStateFlushed.connect(std::bind(&CValidationInterface::ChainStateFlushed, pwalletIn, std::placeholders::_1));
-    conns.BlockChecked = g_signals.m_internals->BlockChecked.connect(std::bind(&CValidationInterface::BlockChecked, pwalletIn, std::placeholders::_1, std::placeholders::_2));
-    conns.NewPoWValidBlock = g_signals.m_internals->NewPoWValidBlock.connect(std::bind(&CValidationInterface::NewPoWValidBlock, pwalletIn, std::placeholders::_1, std::placeholders::_2));
+    g_signals.m_internals->Register(std::move(pwalletIn));
 }
 
 void RegisterValidationInterface(CValidationInterface* callbacks)
@@ -103,7 +134,7 @@ void UnregisterSharedValidationInterface(std::shared_ptr<CValidationInterface> c
 
 void UnregisterValidationInterface(CValidationInterface* pwalletIn) {
     if (g_signals.m_internals) {
-        g_signals.m_internals->m_connMainSignals.erase(pwalletIn);
+        g_signals.m_internals->Unregister(pwalletIn);
     }
 }
 
@@ -111,7 +142,7 @@ void UnregisterAllValidationInterfaces() {
     if (!g_signals.m_internals) {
         return;
     }
-    g_signals.m_internals->m_connMainSignals.clear();
+    g_signals.m_internals->Clear();
 }
 
 void CallFunctionInValidationInterfaceQueue(std::function<void ()> func) {
@@ -151,7 +182,7 @@ void CMainSignals::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockInd
     // in the same critical section where the chain is updated
 
     auto event = [pindexNew, pindexFork, fInitialDownload, this] {
-        m_internals->UpdatedBlockTip(pindexNew, pindexFork, fInitialDownload);
+        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.UpdatedBlockTip(pindexNew, pindexFork, fInitialDownload); });
     };
     ENQUEUE_AND_LOG_EVENT(event, "%s: new block hash=%s fork block hash=%s (in IBD=%s)", __func__,
                           pindexNew->GetBlockHash().ToString(),
@@ -161,7 +192,7 @@ void CMainSignals::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockInd
 
 void CMainSignals::TransactionAddedToMempool(const CTransactionRef &ptx) {
     auto event = [ptx, this] {
-        m_internals->TransactionAddedToMempool(ptx);
+        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionAddedToMempool(ptx); });
     };
     ENQUEUE_AND_LOG_EVENT(event, "%s: txid=%s wtxid=%s", __func__,
                           ptx->GetHash().ToString(),
@@ -170,7 +201,7 @@ void CMainSignals::TransactionAddedToMempool(const CTransactionRef &ptx) {
 
 void CMainSignals::TransactionRemovedFromMempool(const CTransactionRef &ptx) {
     auto event = [ptx, this] {
-        m_internals->TransactionRemovedFromMempool(ptx);
+        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionRemovedFromMempool(ptx); });
     };
     ENQUEUE_AND_LOG_EVENT(event, "%s: txid=%s wtxid=%s", __func__,
                           ptx->GetHash().ToString(),
@@ -179,7 +210,7 @@ void CMainSignals::TransactionRemovedFromMempool(const CTransactionRef &ptx) {
 
 void CMainSignals::BlockConnected(const std::shared_ptr<const CBlock> &pblock, const CBlockIndex *pindex) {
     auto event = [pblock, pindex, this] {
-        m_internals->BlockConnected(pblock, pindex);
+        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.BlockConnected(pblock, pindex); });
     };
     ENQUEUE_AND_LOG_EVENT(event, "%s: block hash=%s block height=%d", __func__,
                           pblock->GetHash().ToString(),
@@ -189,7 +220,7 @@ void CMainSignals::BlockConnected(const std::shared_ptr<const CBlock> &pblock, c
 void CMainSignals::BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindex)
 {
     auto event = [pblock, pindex, this] {
-        m_internals->BlockDisconnected(pblock, pindex);
+        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.BlockDisconnected(pblock, pindex); });
     };
     ENQUEUE_AND_LOG_EVENT(event, "%s: block hash=%s block height=%d", __func__,
                           pblock->GetHash().ToString(),
@@ -198,7 +229,7 @@ void CMainSignals::BlockDisconnected(const std::shared_ptr<const CBlock>& pblock
 
 void CMainSignals::ChainStateFlushed(const CBlockLocator &locator) {
     auto event = [locator, this] {
-        m_internals->ChainStateFlushed(locator);
+        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.ChainStateFlushed(locator); });
     };
     ENQUEUE_AND_LOG_EVENT(event, "%s: block hash=%s", __func__,
                           locator.IsNull() ? "null" : locator.vHave.front().ToString());
@@ -207,10 +238,10 @@ void CMainSignals::ChainStateFlushed(const CBlockLocator &locator) {
 void CMainSignals::BlockChecked(const CBlock& block, const BlockValidationState& state) {
     LOG_EVENT("%s: block hash=%s state=%s", __func__,
               block.GetHash().ToString(), state.ToString());
-    m_internals->BlockChecked(block, state);
+    m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.BlockChecked(block, state); });
 }
 
 void CMainSignals::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr<const CBlock> &block) {
     LOG_EVENT("%s: block hash=%s", __func__, block->GetHash().ToString());
-    m_internals->NewPoWValidBlock(pindex, block);
+    m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.NewPoWValidBlock(pindex, block); });
 }
diff --git a/src/validationinterface.h b/src/validationinterface.h
index f9a359b7ad..cb0204a555 100644
--- a/src/validationinterface.h
+++ b/src/validationinterface.h
@@ -171,9 +171,7 @@ protected:
      * Notifies listeners that a block which builds directly on our current tip
      * has been received and connected to the headers tree, though not validated yet */
     virtual void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr<const CBlock>& block) {};
-    friend void ::RegisterSharedValidationInterface(std::shared_ptr<CValidationInterface>);
-    friend void ::UnregisterValidationInterface(CValidationInterface*);
-    friend void ::UnregisterAllValidationInterfaces();
+    friend class CMainSignals;
 };
 
 struct MainSignalsInstance;