From c30bf9f61ee0ea67269121d99c5de48261232167 Mon Sep 17 00:00:00 2001 From: Mathew Sutcliffe Date: Thu, 16 Oct 2014 16:36:53 +0100 Subject: [PATCH] refs #336 Fixed CLogHandler thread safety in swiftcore. --- src/blackmisc/loghandler.cpp | 5 +---- src/blackmisc/loghandler.h | 10 ++++------ src/blackmisc/worker.h | 21 +++++++++++++++++++++ src/swiftcore/tool.cpp | 10 +++++++--- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/blackmisc/loghandler.cpp b/src/blackmisc/loghandler.cpp index 56e4e7612..a764e5f9c 100644 --- a/src/blackmisc/loghandler.cpp +++ b/src/blackmisc/loghandler.cpp @@ -40,10 +40,7 @@ namespace BlackMisc CLogHandler::~CLogHandler() { - if (m_oldHandler) - { - qInstallMessageHandler(m_oldHandler); - } + qInstallMessageHandler(m_oldHandler); } CLogCategoryHandler *CLogHandler::handlerForCategoryPrefix(const QString &category) diff --git a/src/blackmisc/loghandler.h b/src/blackmisc/loghandler.h index f0645ddc4..a7537c452 100644 --- a/src/blackmisc/loghandler.h +++ b/src/blackmisc/loghandler.h @@ -45,10 +45,6 @@ namespace BlackMisc //! \warning This must only be called from the main thread. CLogCategoryHandler *handlerForCategoryPrefix(const QString &prefix); - //! Enable or disable the default Qt handler. - //! \warning This must only be called from the main thread. - void enableConsoleOutput(bool enable); - signals: //! Emitted when a message is logged in this process. void localMessageLogged(const BlackMisc::CStatusMessage &message); @@ -63,6 +59,9 @@ namespace BlackMisc //! Called by the context to relay a message. void logRemoteMessage(const BlackMisc::CStatusMessage &message); + //! Enable or disable the default Qt handler. + void enableConsoleOutput(bool enable); + private: void logMessage(const BlackMisc::CStatusMessage &message); QtMessageHandler m_oldHandler = nullptr; @@ -80,11 +79,10 @@ namespace BlackMisc { Q_OBJECT - public: + public slots: /*! * Enable or disable the default Qt handler for messages in relevant categories. * This can override the setting of the parent CLogHandler. - * \warning This must only be called from the main thread. */ void enableConsoleOutput(bool enable) { Q_ASSERT(thread() == QThread::currentThread()); m_enableFallThrough = enable; } diff --git a/src/blackmisc/worker.h b/src/blackmisc/worker.h index 9f4bf5417..ee1b92931 100644 --- a/src/blackmisc/worker.h +++ b/src/blackmisc/worker.h @@ -14,12 +14,33 @@ #include #include +#include #include #include namespace BlackMisc { + /*! + * Starts a single-shot timer which will run in an existing thread and call a task when it times out. + * + * Useful when a worker thread wants to push small sub-tasks back to the thread which spawned it. + * \see QTimer::singleShot() + */ + template + void singleShot(int msec, QThread *target, F task) + { + auto *timer = new QTimer; + timer->setSingleShot(true); + timer->moveToThread(target); + QObject::connect(timer, &QTimer::timeout, [ = ]() + { + task(); + timer->deleteLater(); + }); + QMetaObject::invokeMethod(timer, "start", Q_ARG(int, msec)); + } + /*! * Just a subclass of QThread whose destructor waits for the thread to finish. */ diff --git a/src/swiftcore/tool.cpp b/src/swiftcore/tool.cpp index a8c00a1f2..5413009bb 100644 --- a/src/swiftcore/tool.cpp +++ b/src/swiftcore/tool.cpp @@ -4,6 +4,7 @@ #include "blackmisc/avallclasses.h" #include "blackmisc/pqallquantities.h" #include "blackmisc/loghandler.h" +#include "blackmisc/worker.h" #include "blacksound/soundgenerator.h" #include @@ -37,7 +38,7 @@ namespace BlackMiscTest */ void Tool::serverLoop(BlackCore::CRuntime *runtime) { - CLogHandler::instance()->enableConsoleOutput(false); + QMetaObject::invokeMethod(CLogHandler::instance(), "enableConsoleOutput", Q_ARG(bool, false)); Q_ASSERT(runtime); QThread::sleep(3); // let the DBus server startup @@ -164,7 +165,7 @@ namespace BlackMiscTest bool enable = line.endsWith("e"); if (line.startsWith("all")) { - BlackMisc::CLogHandler::instance()->enableConsoleOutput(enable); + QMetaObject::invokeMethod(CLogHandler::instance(), "enableConsoleOutput", Q_ARG(bool, enable)); } else { @@ -177,7 +178,10 @@ namespace BlackMiscTest else if (line.startsWith("sim")) category = CLogCategoryList(simulatorContext).back().toQString(); if (! category.isEmpty()) { - BlackMisc::CLogHandler::instance()->handlerForCategoryPrefix(category)->enableConsoleOutput(enable); + BlackMisc::singleShot(0, BlackMisc::CLogHandler::instance()->thread(), [ = ]() + { + BlackMisc::CLogHandler::instance()->handlerForCategoryPrefix(category)->enableConsoleOutput(enable); + }); } } }