Skip to content

Commit

Permalink
Simplify and fix notifier removal on error.
Browse files Browse the repository at this point in the history
This factors out the common logic to run over all ZMQ notifiers, call a
function on them, and remove them from the list if the function fails is
extracted to a helper method.

Note that this also fixes a potential memory leak:  When a notifier was
removed previously after its callback returned false, it would just be
removed from the list without destructing the object.  This is now done
correctly by std::unique_ptr behind the scenes.
  • Loading branch information
domob1812 committed Sep 7, 2020
1 parent e15b1cf commit b93b9d5
Showing 1 changed file with 24 additions and 26 deletions.
50 changes: 24 additions & 26 deletions src/zmq/zmqnotificationinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,45 +114,43 @@ void CZMQNotificationInterface::Shutdown()
}
}

void CZMQNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload)
{
if (fInitialDownload || pindexNew == pindexFork) // In IBD or blocks were disconnected without any new ones
return;
namespace {

for (auto i = notifiers.begin(); i!=notifiers.end(); )
{
CZMQAbstractNotifier *notifier = i->get();
if (notifier->NotifyBlock(pindexNew))
{
i++;
}
else
{
template <typename Function>
void TryForEachAndRemoveFailed(std::list<std::unique_ptr<CZMQAbstractNotifier>>& notifiers, const Function& func)
{
for (auto i = notifiers.begin(); i != notifiers.end(); ) {
CZMQAbstractNotifier* notifier = i->get();
if (func(notifier)) {
++i;
} else {
notifier->Shutdown();
i = notifiers.erase(i);
}
}
}

} // anonymous namespace

void CZMQNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload)
{
if (fInitialDownload || pindexNew == pindexFork) // In IBD or blocks were disconnected without any new ones
return;

TryForEachAndRemoveFailed(notifiers, [pindexNew](CZMQAbstractNotifier* notifier) {
return notifier->NotifyBlock(pindexNew);
});
}

void CZMQNotificationInterface::TransactionAddedToMempool(const CTransactionRef& ptx)
{
// Used by BlockConnected and BlockDisconnected as well, because they're
// all the same external callback.
const CTransaction& tx = *ptx;

for (auto i = notifiers.begin(); i!=notifiers.end(); )
{
CZMQAbstractNotifier *notifier = i->get();
if (notifier->NotifyTransaction(tx))
{
i++;
}
else
{
notifier->Shutdown();
i = notifiers.erase(i);
}
}
TryForEachAndRemoveFailed(notifiers, [&tx](CZMQAbstractNotifier* notifier) {
return notifier->NotifyTransaction(tx);
});
}

void CZMQNotificationInterface::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected)
Expand Down

0 comments on commit b93b9d5

Please sign in to comment.