From a11a219bbf75e9ff7f2d297228400f863e3560cc Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Mon, 14 Dec 2020 22:40:35 +0100 Subject: [PATCH 1/2] Prevent multiple finish calls on API objects Bad plugin implementations might call finish() more than once which is not expected by receivers of the finished() signal and may cause weird side effects. --- libnymea/integrations/browseractioninfo.cpp | 6 ++++++ libnymea/integrations/browseresult.cpp | 6 ++++++ libnymea/integrations/browseritemactioninfo.cpp | 6 ++++++ libnymea/integrations/browseritemresult.cpp | 6 ++++++ libnymea/integrations/thingactioninfo.cpp | 6 ++++++ libnymea/integrations/thingdiscoveryinfo.cpp | 6 ++++++ libnymea/integrations/thingpairinginfo.cpp | 6 ++++++ libnymea/integrations/thingsetupinfo.cpp | 6 ++++++ 8 files changed, 48 insertions(+) diff --git a/libnymea/integrations/browseractioninfo.cpp b/libnymea/integrations/browseractioninfo.cpp index 935f6eeb..31483956 100644 --- a/libnymea/integrations/browseractioninfo.cpp +++ b/libnymea/integrations/browseractioninfo.cpp @@ -33,6 +33,8 @@ #include +Q_DECLARE_LOGGING_CATEGORY(dcIntegrations) + BrowserActionInfo::BrowserActionInfo(Thing *thing, ThingManager *thingManager, const BrowserAction &browserAction, QObject *parent, quint32 timeout): QObject (parent), m_thing(thing), @@ -85,6 +87,10 @@ QString BrowserActionInfo::translatedDisplayMessage(const QLocale &locale) void BrowserActionInfo::finish(Thing::ThingError status, const QString &displayMessage) { + if (m_finished) { + qCWarning(dcIntegrations()) << "BrowserActionInfo::finish() called on an already finished object."; + return; + } m_finished = true; m_status = status; m_displayMessage = displayMessage; diff --git a/libnymea/integrations/browseresult.cpp b/libnymea/integrations/browseresult.cpp index 93735f31..229bf83c 100644 --- a/libnymea/integrations/browseresult.cpp +++ b/libnymea/integrations/browseresult.cpp @@ -33,6 +33,8 @@ #include +Q_DECLARE_LOGGING_CATEGORY(dcIntegrations) + BrowseResult::BrowseResult(Thing *thing, ThingManager *thingManager, const QString &itemId, const QLocale &locale, QObject *parent, quint32 timeout): QObject(parent), m_thing(thing), @@ -106,6 +108,10 @@ void BrowseResult::addItems(const BrowserItems &items) void BrowseResult::finish(Thing::ThingError status, const QString &displayMessage) { + if (m_finished) { + qCWarning(dcIntegrations()) << "BrowseResult::finish() called on an already finished object."; + return; + } m_finished = true; m_status = status; m_displayMessage = displayMessage; diff --git a/libnymea/integrations/browseritemactioninfo.cpp b/libnymea/integrations/browseritemactioninfo.cpp index 6335801a..a4b3de2c 100644 --- a/libnymea/integrations/browseritemactioninfo.cpp +++ b/libnymea/integrations/browseritemactioninfo.cpp @@ -33,6 +33,8 @@ #include +Q_DECLARE_LOGGING_CATEGORY(dcIntegrations) + BrowserItemActionInfo::BrowserItemActionInfo(Thing *thing, ThingManager *thingManager, const BrowserItemAction &browserItemAction, QObject *parent, quint32 timeout): QObject(parent), m_thing(thing), @@ -85,6 +87,10 @@ QString BrowserItemActionInfo::translatedDisplayMessage(const QLocale &locale) void BrowserItemActionInfo::finish(Thing::ThingError status, const QString &displayMessage) { + if (m_finished) { + qCWarning(dcIntegrations()) << "BrowserItemActionInfo::finish() called on an already finished object."; + return; + } m_finished = true; m_status = status; m_displayMessage = displayMessage; diff --git a/libnymea/integrations/browseritemresult.cpp b/libnymea/integrations/browseritemresult.cpp index a028feac..5891eb0e 100644 --- a/libnymea/integrations/browseritemresult.cpp +++ b/libnymea/integrations/browseritemresult.cpp @@ -33,6 +33,8 @@ #include +Q_DECLARE_LOGGING_CATEGORY(dcIntegrations) + BrowserItemResult::BrowserItemResult(Thing *thing, ThingManager *thingManager, const QString &itemId, const QLocale &locale, QObject *parent, quint32 timeout): QObject(parent), m_thing(thing), @@ -102,6 +104,10 @@ void BrowserItemResult::finish(const BrowserItem &item) void BrowserItemResult::finish(Thing::ThingError status, const QString &displayMessage) { + if (m_finished) { + qCWarning(dcIntegrations()) << "BrowserItemResult::finish() called on an already finished object."; + return; + } m_finished = true; m_status = status; m_displayMessage = displayMessage; diff --git a/libnymea/integrations/thingactioninfo.cpp b/libnymea/integrations/thingactioninfo.cpp index 377f817b..92669e01 100644 --- a/libnymea/integrations/thingactioninfo.cpp +++ b/libnymea/integrations/thingactioninfo.cpp @@ -34,6 +34,8 @@ #include +Q_DECLARE_LOGGING_CATEGORY(dcIntegrations) + ThingActionInfo::ThingActionInfo(Thing *thing, const Action &action, ThingManager *parent, quint32 timeout): QObject(parent), m_thing(thing), @@ -85,6 +87,10 @@ QString ThingActionInfo::translatedDisplayMessage(const QLocale &locale) void ThingActionInfo::finish(Thing::ThingError status, const QString &displayMessage) { + if (m_finished) { + qCWarning(dcIntegrations()) << "ThingActionInfo::finish() called on an already finished object."; + return; + } m_finished = true; m_status = status; m_displayMessage = displayMessage; diff --git a/libnymea/integrations/thingdiscoveryinfo.cpp b/libnymea/integrations/thingdiscoveryinfo.cpp index 99bc6163..e8b1067f 100644 --- a/libnymea/integrations/thingdiscoveryinfo.cpp +++ b/libnymea/integrations/thingdiscoveryinfo.cpp @@ -33,6 +33,8 @@ #include +Q_DECLARE_LOGGING_CATEGORY(dcIntegrations) + ThingDiscoveryInfo::ThingDiscoveryInfo(const ThingClassId &thingClassId, const ParamList ¶ms, ThingManager *thingManager, quint32 timeout): QObject(thingManager), m_thingClassId(thingClassId), @@ -100,6 +102,10 @@ QString ThingDiscoveryInfo::translatedDisplayMessage(const QLocale &locale) void ThingDiscoveryInfo::finish(Thing::ThingError status, const QString &displayMessage) { + if (m_finished) { + qCWarning(dcIntegrations()) << "ThingDiscoveryInfo::finish() called on an already finished object."; + return; + } m_finished = true; m_status = status; m_displayMessage = displayMessage; diff --git a/libnymea/integrations/thingpairinginfo.cpp b/libnymea/integrations/thingpairinginfo.cpp index 761ff879..c377f411 100644 --- a/libnymea/integrations/thingpairinginfo.cpp +++ b/libnymea/integrations/thingpairinginfo.cpp @@ -33,6 +33,8 @@ #include +Q_DECLARE_LOGGING_CATEGORY(dcIntegrations) + ThingPairingInfo::ThingPairingInfo(const PairingTransactionId &pairingTransactionId, const ThingClassId &thingClassId, const ThingId &thingId, const QString &deviceName, const ParamList ¶ms, const ThingId &parentId, ThingManager *parent, quint32 timeout): QObject(parent), m_transactionId(pairingTransactionId), @@ -113,6 +115,10 @@ QString ThingPairingInfo::translatedDisplayMessage(const QLocale &locale) const void ThingPairingInfo::finish(Thing::ThingError status, const QString &displayMessage) { + if (m_finished) { + qCWarning(dcIntegrations()) << "ThingPairingInfo::finish() called on an already finished object."; + return; + } m_finished = true; m_status = status; m_displayMessage = displayMessage; diff --git a/libnymea/integrations/thingsetupinfo.cpp b/libnymea/integrations/thingsetupinfo.cpp index cf75921d..cebb3bf9 100644 --- a/libnymea/integrations/thingsetupinfo.cpp +++ b/libnymea/integrations/thingsetupinfo.cpp @@ -35,6 +35,8 @@ #include +Q_DECLARE_LOGGING_CATEGORY(dcIntegrations) + ThingSetupInfo::ThingSetupInfo(Thing *thing, ThingManager *thingManager, quint32 timeout): QObject(thingManager), m_thing(thing), @@ -81,6 +83,10 @@ QString ThingSetupInfo::translatedDisplayMessage(const QLocale &locale) void ThingSetupInfo::finish(Thing::ThingError status, const QString &displayMessage) { + if (m_finished) { + qCWarning(dcIntegrations()) << "ThingSetupInfo::finish() called on an already finished object."; + return; + } m_finished = true; m_status = status; m_displayMessage = displayMessage; From f1fa59c5351877ef4701df28b7e988f1a2e7c555 Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Mon, 14 Dec 2020 23:03:55 +0100 Subject: [PATCH 2/2] Fix initialisation of the global logging category variable Just accessing the raw variable doesn't guarantee the order of construction and may lead to crashes in certain constellations. --- libnymea-core/nymeacore.cpp | 2 +- libnymea/integrations/integrationplugin.cpp | 3 +++ libnymea/loggingcategories.cpp | 6 ++++-- libnymea/loggingcategories.h | 5 ++--- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/libnymea-core/nymeacore.cpp b/libnymea-core/nymeacore.cpp index 48c144c1..0f3b42c9 100644 --- a/libnymea-core/nymeacore.cpp +++ b/libnymea-core/nymeacore.cpp @@ -601,7 +601,7 @@ QStringList NymeaCore::getAvailableLanguages() QStringList NymeaCore::loggingFilters() { - return s_nymeaLoggingCategories; + return nymeaLoggingCategories(); } QStringList NymeaCore::loggingFiltersPlugins() diff --git a/libnymea/integrations/integrationplugin.cpp b/libnymea/integrations/integrationplugin.cpp index 75923321..f034bc05 100644 --- a/libnymea/integrations/integrationplugin.cpp +++ b/libnymea/integrations/integrationplugin.cpp @@ -100,6 +100,9 @@ /*! IntegrationPlugin constructor. IntegrationPlugins will be instantiated by the system. This should never be called manually by a plugin implementation. */ + +NYMEA_LOGGING_CATEGORY(dcIntegrations, "Integrations") + IntegrationPlugin::IntegrationPlugin(QObject *parent): QObject(parent) { diff --git a/libnymea/loggingcategories.cpp b/libnymea/loggingcategories.cpp index 1f1fe6ab..fc807fb8 100644 --- a/libnymea/loggingcategories.cpp +++ b/libnymea/loggingcategories.cpp @@ -34,10 +34,12 @@ #include #include -QStringList s_nymeaLoggingCategories; +QStringList& nymeaLoggingCategories() { + static QStringList _nymeaLoggingCategories; + return _nymeaLoggingCategories; +} // FIXME: Those should eventually disappear from here -NYMEA_LOGGING_CATEGORY(dcIntegrations, "Integrations"); NYMEA_LOGGING_CATEGORY(dcThing, "Thing") NYMEA_LOGGING_CATEGORY(dcThingManager, "ThingManager") NYMEA_LOGGING_CATEGORY(dcSystem, "System") diff --git a/libnymea/loggingcategories.h b/libnymea/loggingcategories.h index fdb991a2..1fc74cec 100644 --- a/libnymea/loggingcategories.h +++ b/libnymea/loggingcategories.h @@ -34,13 +34,12 @@ #include #include -extern QStringList s_nymeaLoggingCategories; - +QStringList& nymeaLoggingCategories(); #define NYMEA_LOGGING_CATEGORY(name, string) \ class NymeaLoggingCategory##name: public QLoggingCategory { \ public: \ - NymeaLoggingCategory##name(): QLoggingCategory(string) { s_nymeaLoggingCategories.append(string); } \ + NymeaLoggingCategory##name(): QLoggingCategory(string) { nymeaLoggingCategories().append(string); } \ }; \ static NymeaLoggingCategory##name s_##name; \ const QLoggingCategory &name() \