From b12562ec5f8894e9afe3459d4be2d8d9ac4f1b43 Mon Sep 17 00:00:00 2001
From: Mathew Sutcliffe <oktal3700@gmail.com>
Date: Tue, 21 Oct 2014 01:24:41 +0100
Subject: [PATCH] refs #338 Fixed reentrancy of CLogHandler garbage collection.

---
 src/blackmisc/loghandler.cpp | 17 ++++++++++++-----
 src/blackmisc/loghandler.h   | 24 ++++++++----------------
 2 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/src/blackmisc/loghandler.cpp b/src/blackmisc/loghandler.cpp
index a764e5f9c..af91d0fcf 100644
--- a/src/blackmisc/loghandler.cpp
+++ b/src/blackmisc/loghandler.cpp
@@ -51,11 +51,6 @@ namespace BlackMisc
         {
             m_categoryPrefixHandlers[category] = new CLogCategoryHandler(this, m_enableFallThrough);
 
-            connect(m_categoryPrefixHandlers[category], &CLogCategoryHandler::ps_canBeDeleted, [this](CLogCategoryHandler *handler)
-            {
-                m_categoryPrefixHandlers.remove(m_categoryPrefixHandlers.key(handler));
-                QMetaObject::invokeMethod(handler, "deleteLater");
-            });
         }
 
         return m_categoryPrefixHandlers[category];
@@ -111,6 +106,8 @@ namespace BlackMisc
 
     void CLogHandler::logMessage(const CStatusMessage &statusMessage)
     {
+        collectGarbage();
+
         auto handlers = handlersForCategories(statusMessage.getCategories());
 
         if (isFallThroughEnabled(handlers))
@@ -128,4 +125,14 @@ namespace BlackMisc
         }
     }
 
+    void CLogHandler::collectGarbage()
+    {
+        auto newEnd = std::stable_partition(m_patternHandlers.begin(), m_patternHandlers.end(), [](const PatternPair &pair)
+        {
+            return ! pair.second->canBeDeleted();
+        });
+        std::for_each(newEnd, m_patternHandlers.end(), [](const PatternPair &pair) { pair.second->deleteLater(); });
+        m_patternHandlers.erase(newEnd, m_patternHandlers.end());
+    }
+
 }
diff --git a/src/blackmisc/loghandler.h b/src/blackmisc/loghandler.h
index a7537c452..88e2ccca6 100644
--- a/src/blackmisc/loghandler.h
+++ b/src/blackmisc/loghandler.h
@@ -64,6 +64,7 @@ namespace BlackMisc
 
     private:
         void logMessage(const BlackMisc::CStatusMessage &message);
+        void collectGarbage();
         QtMessageHandler m_oldHandler = nullptr;
         bool m_enableFallThrough = true;
         bool isFallThroughEnabled(const QList<CLogCategoryHandler *> &handlers) const;
@@ -90,7 +91,7 @@ namespace BlackMisc
         /*!
          * Emitted when a message is logged in a relevant category.
          *
-         * When all slots are disconnected from this signal, the CLogCategoryHandler is allowed to delete itself.
+         * When all slots are disconnected from this signal, the CLogCategoryHandler could be deleted.
          *
          * Note that if a message matches more that one category handler, then this signal will be emitted for all of them,
          * so if a slot is connected to all of them then it will be called multiple times. Use the methods
@@ -100,25 +101,16 @@ namespace BlackMisc
          */
         void messageLogged(const CStatusMessage &message);
 
-        /*!
-         * Emitted when there are no more slots connected to the messageLogged signal.
-         */
-        void ps_canBeDeleted(CLogCategoryHandler *handler);
-
-    protected:
-        /*!
-         * \copydoc QObject::disconnectNotify
-         */
-        virtual void disconnectNotify(const QMetaMethod &) override
-        {
-            static const QMetaMethod signal = QMetaMethod::fromSignal(&CLogCategoryHandler::messageLogged);
-            if (! isSignalConnected(signal)) { emit ps_canBeDeleted(this); }
-        }
-
     private:
         friend class CLogHandler;
         CLogCategoryHandler(QObject *parent, bool enableFallThrough) : QObject(parent), m_enableFallThrough(enableFallThrough) {}
         bool m_enableFallThrough;
+
+        bool canBeDeleted()
+        {
+            static const auto signal = QMetaMethod::fromSignal(&CLogPatternHandler::messageLogged);
+            return ! isSignalConnected(signal);
+        }
     };
 }