refs #522, mutex for mutable members (thread safe)

* copy/assignment/lock for mutable members
* also removed redundant in status message (slack discussion MS/KB)

src/blackgui/managedstatusbar.cpp

@@ -92,7 +92,6 @@ namespace BlackGui
         Q_ASSERT_X(this->m_statusBarIcon, Q_FUNC_INFO, "Missing status bar icon");
         Q_ASSERT_X(this->m_statusBar, Q_FUNC_INFO, "Missing status bar");
 
-        if (statusMessage.isRedundant()) { return; }
         if (statusMessage.wasHandledBy(this)) { return; }
         statusMessage.markAsHandledBy(this);
 

src/blackmisc/logmessage.cpp

@@ -64,7 +64,6 @@ namespace BlackMisc
 
     CLogMessage::operator CStatusMessage()
     {
-        m_redundant = true;
         return { m_categories, m_severity, message() };
     }
 
@@ -95,7 +94,6 @@ namespace BlackMisc
         {
             QString category = m_categories.toQString();
             if (m_severity == CStatusMessage::SeverityDebug) { category = CLogMessageHelper::addDebugFlag(category); }
-            if (m_redundant) { category = CLogMessageHelper::addRedundantFlag(category); }
             return category.toLatin1();
         }
     }
@@ -167,10 +165,8 @@ namespace BlackMisc
         if (category.isEmpty() || hasFlag(category, flag)) return category;
         return category + "/" + flag;
     }
-    QString CLogMessageHelper::addRedundantFlag(const QString &category) { return addFlag(category, "redundant"); }
     QString CLogMessageHelper::addDebugFlag(const QString &category) { return addFlag(category, "debug"); }
     QString CLogMessageHelper::stripFlags(const QString &category) { return category.section("/", 0, 1); }
-    bool CLogMessageHelper::hasRedundantFlag(const QString &category) { return hasFlag(category, "redundant"); }
     bool CLogMessageHelper::hasDebugFlag(const QString &category)
     {
         return hasFlag(category, "debug") || category.isEmpty()
@@ -191,5 +187,4 @@ namespace BlackMisc
             preformatted(msg);
         }
     }
-
+} // ns
-}

src/blackmisc/logmessage.h

@@ -67,17 +67,11 @@ namespace BlackMisc
         //! Returns an encoded category string with the debug flag appended.
         static QString addDebugFlag(const QString &category);
 
-        //! Returns an encoded category string with the redundant flag appended.
-        static QString addRedundantFlag(const QString &category);
-
         //! Strips all flags from an encoded category string, returning only the plain category string.
         static QString stripFlags(const QString &category);
 
         //! Returns true if the given encoded category string has the debug flag.
         static bool hasDebugFlag(const QString &category);
-
-        //! Returns true if the given encoded category string has the redundant flag.
-        static bool hasRedundantFlag(const QString &category);
     };
 
     /*!
@@ -173,7 +167,6 @@ namespace BlackMisc
         CLogCategoryList m_categories = CLogCategoryList { CLogCategory::uncategorized() };
         QString m_message;
         QStringList m_args;
-        bool m_redundant = false;
 
         CLogMessage &arg(QString value) { m_args.push_back(value); return *this; }
         QString message() const;

src/blackmisc/network/urllist.cpp

@@ -19,6 +19,24 @@ namespace BlackMisc
     {
         CUrlList::CUrlList() { }
 
+        CUrlList::CUrlList(const CUrlList &other) : CSequence<CUrl>(other)
+        {
+            *this = other;
+        }
+
+        CUrlList &CUrlList::operator =(const CUrlList &other)
+        {
+            if (this == &other) { return *this; }
+
+            QReadLocker readLock(&other.m_lock);
+            int index = other.m_currentIndexDistributedLoad;
+            readLock.unlock(); // avoid deadlock
+
+            QWriteLocker writeLock(&this->m_lock);
+            this->m_currentIndexDistributedLoad = index;
+            return *this;
+        }
+
         CUrlList::CUrlList(const QStringList &listOfUrls, bool removeDuplicates)
         {
             QStringList urlList(listOfUrls);

src/blackmisc/network/urllist.h

@@ -16,6 +16,7 @@
 #include "blackmisc/network/url.h"
 #include "blackmisc/collection.h"
 #include "blackmisc/sequence.h"
+#include <QReadWriteLock>
 
 namespace BlackMisc
 {
@@ -32,6 +33,12 @@ namespace BlackMisc
             //! Default constructor.
             CUrlList();
 
+            //! Copy constructor (because of mutex)
+            CUrlList(const CUrlList &other);
+
+            //! Copy assignment (because of mutex)
+            CUrlList &operator =(const CUrlList &other);
+
             //! By list of URLs
             explicit CUrlList(const QStringList &listOfUrls, bool removeDuplicates = true);
 
@@ -67,6 +74,7 @@ namespace BlackMisc
 
         private:
             mutable int m_currentIndexDistributedLoad = -1; //!< index for random access
+            mutable QReadWriteLock m_lock;  //!< lock (because of mutable members)
         };
 
         //! URL list with fail support

src/blackmisc/pixmap.cpp

@@ -13,14 +13,12 @@
 
 namespace BlackMisc
 {
-    CPixmap::CPixmap() = default;
-
     CPixmap::CPixmap(const QPixmap &pixmap) : m_pixmap(pixmap), m_hasCachedPixmap(true)
     {
         this->fillByteArray();
     }
 
-    CPixmap::CPixmap(const CPixmap &other) : CValueObject()
+    CPixmap::CPixmap(const CPixmap &other) : CValueObject(other)
     {
         *this = other;
     }

src/blackmisc/pixmap.h

@@ -25,7 +25,7 @@ namespace BlackMisc
     {
     public:
         //! Default constructor.
-        CPixmap();
+        CPixmap() = default;
 
         //! Constructor.
         CPixmap(const QPixmap &pixmap);
@@ -57,11 +57,10 @@ namespace BlackMisc
         //! Init the byte array with data
         void fillByteArray();
 
+        QByteArray m_array;                     //!< data of pixmap
         mutable QPixmap m_pixmap;               //!< cached pixmap, mutable because of lazy initialization
         mutable bool m_hasCachedPixmap = false; //!< pixmap? Mutable because of lazy initialization
         mutable QReadWriteLock m_lock;          //!< lock (because of mutable members)
-
-        QByteArray m_array;                     //!< data of pixmap
     };
 } // namespace
 

src/blackmisc/statusexception.cpp

@@ -0,0 +1,30 @@
+/* Copyright (C) 2015
+ * swift project Community / Contributors
+ *
+ * This file is part of Swift Project. It is subject to the license terms in the LICENSE file found in the top-level
+ * directory of this distribution and at http://www.swift-project.org/license.html. No part of swift project,
+ * including this file, may be copied, modified, propagated, or distributed except according to the terms
+ * contained in the LICENSE file.
+ */
+
+#include "statusexception.h"
+
+namespace BlackMisc
+{
+    CStatusException::CStatusException(const CStatusMessage &payload) :
+        m_payload(payload)
+    {}
+
+    CStatusException::CStatusException(const CStatusException &other) : std::exception(other)
+    {
+        QReadLocker lock(&other.m_lock);
+        this->m_temp = other.m_temp;
+    }
+
+    const char *CStatusException::what() const Q_DECL_NOEXCEPT
+    {
+        QWriteLocker lock(&this->m_lock);
+        if (m_temp.isNull()) { m_temp = m_payload.getMessage().toLocal8Bit(); }
+        return m_temp;
+    }
+} // ns

src/blackmisc/statusexception.h

@@ -1,5 +1,5 @@
 /* Copyright (C) 2015
- * Swift Project Community / Contributors
+ * swift Project Community / Contributors
  *
  * This file is part of Swift Project. It is subject to the license terms in the LICENSE file found in the top-level
  * directory of this distribution and at http://www.swift-project.org/license.html. No part of swift project,
@@ -17,7 +17,6 @@
 
 namespace BlackMisc
 {
-
     /*!
      * Throwable exception class containing a CStatusMessage.
      *
@@ -27,21 +26,19 @@ namespace BlackMisc
     {
     public:
         //! Constructor.
-        explicit CStatusException(const CStatusMessage &payload) :
+        explicit CStatusException(const CStatusMessage &payload);
-            m_payload(payload)
+
-        {}
+        //! Copy constructor (because of mutex)
+        CStatusException(const CStatusException &other);
+
+        //! Copy assignment (because of mutex)
+        CStatusException &operator=(const CStatusException &) = delete;
 
         //! Return null-terminated message string.
-        virtual const char *what() const Q_DECL_NOEXCEPT override
+        virtual const char *what() const Q_DECL_NOEXCEPT override;
-        {
-            return m_temp = m_payload.getMessage().toLocal8Bit();
-        }
 
         //! Return the contained status message.
-        const CStatusMessage &status() const
+        const CStatusMessage &status() const { return m_payload; }
-        {
-            return m_payload;
-        }
 
         //! Destructor.
         ~CStatusException() Q_DECL_NOEXCEPT {}
@@ -49,8 +46,8 @@ namespace BlackMisc
     private:
         const CStatusMessage   m_payload;
         mutable QByteArray     m_temp;
+        mutable QReadWriteLock m_lock;  //!< lock (because of mutable members)
     };
-
+} // ns
-}
 
 #endif

src/blackmisc/statusmessage.cpp

@@ -18,10 +18,24 @@
 
 namespace BlackMisc
 {
-    void CStatusMessage::registerMetadata()
+    CStatusMessage::CStatusMessage(const CStatusMessage &other) :
+        CValueObject(other),
+        ITimestampBased(other)
     {
-        CValueObject<CStatusMessage>::registerMetadata();
+        *this = other;
-        qRegisterMetaType<StatusSeverity>();
+    }
+
+    CStatusMessage &CStatusMessage::operator =(const CStatusMessage &other)
+    {
+        if (this == &other) { return *this; }
+
+        QReadLocker readLock(&other.m_lock);
+        auto tuple = std::make_tuple(other.m_categories, other.m_severity, other.m_message, other.m_handledByObjects);
+        readLock.unlock(); // avoid deadlock
+
+        QWriteLocker writeLock(&this->m_lock);
+        std::tie(m_categories, m_severity, m_message, m_handledByObjects) = tuple;
+        return *this;
     }
 
     CStatusMessage::CStatusMessage(const QString &message)
@@ -39,7 +53,6 @@ namespace BlackMisc
     CStatusMessage::CStatusMessage(QtMsgType type, const QMessageLogContext &context, const QString &message)
         : CStatusMessage(message)
     {
-        m_redundant = CLogMessageHelper::hasRedundantFlag(context.category);
         bool debug = CLogMessageHelper::hasDebugFlag(context.category);
         auto categories = CLogMessageHelper::stripFlags(context.category);
         m_categories = CLogCategoryList::fromQString(categories);
@@ -67,10 +80,6 @@ namespace BlackMisc
         {
             category = CLogMessageHelper::addDebugFlag(category);
         }
-        if (this->m_redundant)
-        {
-            category = CLogMessageHelper::addRedundantFlag(category);
-        }
 
         *o_category = category;
         *o_message = this->m_message;
@@ -105,16 +114,11 @@ namespace BlackMisc
     }
 
     QString CStatusMessage::getHumanReadablePattern() const
-    {
-        //! \todo This should me not hardcoded
-        if (this->m_humanReadableCategory.isEmpty())
     {
         QStringList patternNames(getHumanReadablePatterns());
-            this->m_humanReadableCategory = patternNames.isEmpty() ?
+        return patternNames.isEmpty() ?
                "None" : patternNames.join(", ");
     }
-        return this->m_humanReadableCategory;
-    }
 
     QStringList CStatusMessage::getHumanReadablePatterns() const
     {
@@ -183,6 +187,12 @@ namespace BlackMisc
         return m;
     }
 
+    void CStatusMessage::registerMetadata()
+    {
+        CValueObject<CStatusMessage>::registerMetadata();
+        qRegisterMetaType<StatusSeverity>();
+    }
+
     CStatusMessage::StatusSeverity CStatusMessage::stringToSeverity(const QString &severity)
     {
         // pre-check
@@ -339,4 +349,4 @@ namespace BlackMisc
         html.append("</font>");
         return html;
     }
-}
+} // ns

src/blackmisc/statusmessage.h

@@ -17,10 +17,10 @@
 #include "propertyindex.h"
 #include "logcategorylist.h"
 #include "timestampbased.h"
+#include <QReadWriteLock>
 
 namespace BlackMisc
 {
-
     class CStatusException;
 
     /*!
@@ -50,12 +50,15 @@ namespace BlackMisc
             IndexMessage
         };
 
-        //! \copydoc BlackMisc::CValueObject::registerMetadata
-        static void registerMetadata();
-
         //! Constructor
         CStatusMessage() = default;
 
+        //! Copy constructor (because of mutex)
+        CStatusMessage(const CStatusMessage &other);
+
+        //! Copy assignment (because of mutex)
+        CStatusMessage &operator =(const CStatusMessage &other);
+
         //! Constructor
         CStatusMessage(const QString &message);
 
@@ -97,9 +100,6 @@ namespace BlackMisc
         //! Message empty
         bool isEmpty() const { return this->m_message.isEmpty(); }
 
-        //! Message may already have been handled directly
-        bool isRedundant() const { return this->m_redundant; }
-
         //! Info or debug, no warning or error
         bool isSeverityInfoOrLess() const { return this->m_severity == SeverityInfo || this->m_severity == SeverityDebug; }
 
@@ -111,9 +111,6 @@ namespace BlackMisc
             return std::all_of(classCategories.begin(), classCategories.end(), [this](const CLogCategory & cat) { return m_categories.contains(cat); });
         }
 
-        //! Mark the message as potentially already handled
-        void markAsRedundant() { this->m_redundant = true; }
-
         //! Mark the message as having been handled by the given object
         void markAsHandledBy(const QObject *object) const;
 
@@ -168,15 +165,16 @@ namespace BlackMisc
         //! Object from JSON
         static CStatusMessage fromDatabaseJson(const QJsonObject &json);
 
+        //! \copydoc BlackMisc::CValueObject::registerMetadata
+        static void registerMetadata();
+
     private:
         BLACK_ENABLE_TUPLE_CONVERSION(CStatusMessage)
         CLogCategoryList          m_categories;
         StatusSeverity            m_severity = SeverityDebug;
         QString                   m_message;
-        bool             m_redundant = false;
         mutable QVector<quintptr> m_handledByObjects;
-        mutable QString           m_humanReadableCategory; //!< human readable category cache
+        mutable QReadWriteLock    m_lock;  //!< lock (because of mutable members)
-
     };
 } // namespace
 
@@ -186,7 +184,6 @@ BLACK_DECLARE_TUPLE_CONVERSION(BlackMisc::CStatusMessage, (
                                    o.m_severity,
                                    o.m_message,
                                    o.m_timestampMSecsSinceEpoch,
-                                   o.m_redundant,
                                    attr(o.m_handledByObjects, flags < DisabledForHashing | DisabledForJson | DisabledForComparison | DisabledForMarshalling > ())
                                ))
 Q_DECLARE_METATYPE(BlackMisc::CStatusMessage)

src/blackmisc/valuecache.cpp

@@ -406,7 +406,6 @@ namespace BlackMisc
         else
         {
             CLogMessage::preformatted(error);
-            error.markAsRedundant();
         }
         return error;
     }

src/swiftguistandard/swiftguistd.cpp

@@ -197,7 +197,6 @@ bool SwiftGuiStd::isContextAudioAvailableCheck()
 void SwiftGuiStd::ps_displayStatusMessageInGui(const CStatusMessage &statusMessage)
 {
     if (!this->m_init) { return; }
-    if (statusMessage.isRedundant()) { return; }
     if (statusMessage.wasHandledBy(this)) { return; }
     statusMessage.markAsHandledBy(this);
     this->m_statusBar.displayStatusMessage(statusMessage);