From b93b9d54569145bfcec6cee10968284fe05fe254 Mon Sep 17 00:00:00 2001
From: Daniel Kraft <d@domob.eu>
Date: Tue, 1 Sep 2020 09:40:43 +0200
Subject: [PATCH] Simplify and fix notifier removal on error.

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.
---
 src/zmq/zmqnotificationinterface.cpp | 50 +++++++++++++---------------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp
index 96e5b3c0390..449651e65f6 100644
--- a/src/zmq/zmqnotificationinterface.cpp
+++ b/src/zmq/zmqnotificationinterface.cpp
@@ -114,24 +114,32 @@ void CZMQNotificationInterface::Shutdown()
     }
 }
 
+namespace {
+
+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;
 
-    for (auto i = notifiers.begin(); i!=notifiers.end(); )
-    {
-        CZMQAbstractNotifier *notifier = i->get();
-        if (notifier->NotifyBlock(pindexNew))
-        {
-            i++;
-        }
-        else
-        {
-            notifier->Shutdown();
-            i = notifiers.erase(i);
-        }
-    }
+    TryForEachAndRemoveFailed(notifiers, [pindexNew](CZMQAbstractNotifier* notifier) {
+        return notifier->NotifyBlock(pindexNew);
+    });
 }
 
 void CZMQNotificationInterface::TransactionAddedToMempool(const CTransactionRef& ptx)
@@ -140,19 +148,9 @@ void CZMQNotificationInterface::TransactionAddedToMempool(const CTransactionRef&
     // 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)