diff --git a/src/validationinterface.h b/src/validationinterface.h --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -163,10 +163,7 @@ */ virtual void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr &block){}; - friend void ::RegisterSharedValidationInterface( - std::shared_ptr); - friend void ::UnregisterValidationInterface(CValidationInterface *); - friend void ::UnregisterAllValidationInterfaces(); + friend class CMainSignals; }; struct MainSignalsInstance; diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -18,49 +18,80 @@ #include #include -#include - -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 +//! 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 - UpdatedBlockTip; - boost::signals2::signal - TransactionAddedToMempool; - boost::signals2::signal &, - const CBlockIndex *pindex, - const std::vector &)> - BlockConnected; - boost::signals2::signal &)> - BlockDisconnected; - boost::signals2::signal - TransactionRemovedFromMempool; - boost::signals2::signal ChainStateFlushed; - boost::signals2::signal - BlockChecked; - boost::signals2::signal &)> - 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 callbacks; + int count = 1; + }; + std::list m_list GUARDED_BY(m_mutex); + std::unordered_map::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 - m_connMainSignals; explicit MainSignalsInstance(CScheduler *pscheduler) : m_schedulerClient(pscheduler) {} + + void Register(std::shared_ptr 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 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; @@ -95,36 +126,7 @@ std::shared_ptr 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, - std::placeholders::_3)); - conns.BlockDisconnected = g_signals.m_internals->BlockDisconnected.connect( - std::bind(&CValidationInterface::BlockDisconnected, pwalletIn, - std::placeholders::_1)); - 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) { @@ -141,7 +143,7 @@ void UnregisterValidationInterface(CValidationInterface *pwalletIn) { if (g_signals.m_internals) { - g_signals.m_internals->m_connMainSignals.erase(pwalletIn); + g_signals.m_internals->Unregister(pwalletIn); } } @@ -149,7 +151,7 @@ if (!g_signals.m_internals) { return; } - g_signals.m_internals->m_connMainSignals.clear(); + g_signals.m_internals->Clear(); } void CallFunctionInValidationInterfaceQueue(std::function func) { @@ -189,7 +191,9 @@ // 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__, @@ -199,14 +203,20 @@ } void CMainSignals::TransactionAddedToMempool(const CTransactionRef &ptx) { - auto event = [ptx, this] { m_internals->TransactionAddedToMempool(ptx); }; + auto event = [ptx, this] { + m_internals->Iterate([&](CValidationInterface &callbacks) { + callbacks.TransactionAddedToMempool(ptx); + }); + }; ENQUEUE_AND_LOG_EVENT(event, "%s: txid=%s", __func__, ptx->GetHash().ToString()); } 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", __func__, ptx->GetHash().ToString()); @@ -216,7 +226,9 @@ const std::shared_ptr &pblock, const CBlockIndex *pindex, const std::shared_ptr> &pvtxConflicted) { auto event = [pblock, pindex, pvtxConflicted, this] { - m_internals->BlockConnected(pblock, pindex, *pvtxConflicted); + m_internals->Iterate([&](CValidationInterface &callbacks) { + callbacks.BlockConnected(pblock, pindex, *pvtxConflicted); + }); }; ENQUEUE_AND_LOG_EVENT(event, "%s: block hash=%s block height=%d", __func__, pblock->GetHash().ToString(), pindex->nHeight); @@ -226,13 +238,21 @@ const std::shared_ptr &pblock) { // TODO: This function was refactored as part of an out-of-order backport // of https://github.com/bitcoin/bitcoin/pull/16688 - auto event = [pblock, this] { m_internals->BlockDisconnected(pblock); }; + auto event = [pblock, this] { + m_internals->Iterate([&](CValidationInterface &callbacks) { + callbacks.BlockDisconnected(pblock); + }); + }; ENQUEUE_AND_LOG_EVENT(event, "%s: block hash=%s", __func__, pblock->GetHash().ToString()); } void CMainSignals::ChainStateFlushed(const CBlockLocator &locator) { - auto event = [locator, this] { m_internals->ChainStateFlushed(locator); }; + auto event = [locator, this] { + 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()); @@ -242,11 +262,15 @@ const CValidationState &state) { LOG_EVENT("%s: block hash=%s state=%s", __func__, block.GetHash().ToString(), FormatStateMessage(state)); - m_internals->BlockChecked(block, state); + m_internals->Iterate([&](CValidationInterface &callbacks) { + callbacks.BlockChecked(block, state); + }); } void CMainSignals::NewPoWValidBlock( const CBlockIndex *pindex, const std::shared_ptr &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); + }); }