diff options
author | Wladimir J. van der Laan <laanwj@protonmail.com> | 2020-04-06 16:22:22 +0200 |
---|---|---|
committer | Wladimir J. van der Laan <laanwj@protonmail.com> | 2020-04-06 16:46:07 +0200 |
commit | fdeb445a34a97357c87416d80350f9b0b4f222ca (patch) | |
tree | 355eb14bf31fa1c93085b8f15cf2e38dfb0cfb18 | |
parent | 299544f9c5375810f3b4f70e68d2340fe689108a (diff) | |
parent | d6815a2313158862d448733954a73520f223deb6 (diff) | |
download | bitcoin-fdeb445a34a97357c87416d80350f9b0b4f222ca.tar.xz |
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
-rw-r--r-- | src/validationinterface.cpp | 115 | ||||
-rw-r--r-- | src/validationinterface.h | 4 |
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; |