From 978c57f1ed856d6b123b9746f7076bdcea2c55aa Mon Sep 17 00:00:00 2001 From: Julien Wadel Date: Mon, 30 Oct 2023 09:18:29 +0100 Subject: [PATCH] Check running thread and factorize debug class names. --- Linphone/core/App.cpp | 1 + Linphone/core/App.hpp | 3 +- Linphone/core/CMakeLists.txt | 1 - Linphone/core/account/Account.cpp | 4 ++ Linphone/core/account/Account.hpp | 4 +- Linphone/core/account/AccountList.cpp | 11 +++- Linphone/core/account/AccountList.hpp | 5 +- Linphone/core/phone-number/PhoneNumber.cpp | 6 +++ Linphone/core/phone-number/PhoneNumber.hpp | 6 ++- .../core/phone-number/PhoneNumberList.cpp | 15 ++++-- .../core/phone-number/PhoneNumberList.hpp | 6 ++- Linphone/model/account/AccountManager.cpp | 14 ++++- Linphone/model/account/AccountManager.hpp | 5 +- Linphone/model/account/AccountModel.cpp | 5 ++ Linphone/model/account/AccountModel.hpp | 8 ++- Linphone/model/setting/SettingsModel.cpp | 6 ++- Linphone/model/setting/SettingsModel.hpp | 7 ++- Linphone/tool/AbstractObject.hpp | 52 +++++++++++++++++++ Linphone/tool/CMakeLists.txt | 2 +- Linphone/{core => tool}/thread/Thread.cpp | 14 +++++ Linphone/{core => tool}/thread/Thread.hpp | 8 ++- 21 files changed, 164 insertions(+), 19 deletions(-) create mode 100644 Linphone/tool/AbstractObject.hpp rename Linphone/{core => tool}/thread/Thread.cpp (61%) rename Linphone/{core => tool}/thread/Thread.hpp (84%) diff --git a/Linphone/core/App.cpp b/Linphone/core/App.cpp index 626436c56..e37f0b8a4 100644 --- a/Linphone/core/App.cpp +++ b/Linphone/core/App.cpp @@ -34,6 +34,7 @@ #include "core/phone-number/PhoneNumberProxy.hpp" #include "core/singleapplication/singleapplication.h" #include "tool/Constants.hpp" +#include "tool/thread/Thread.hpp" #include "tool/providers/ImageProvider.hpp" diff --git a/Linphone/core/App.hpp b/Linphone/core/App.hpp index aca4f5037..20c764150 100644 --- a/Linphone/core/App.hpp +++ b/Linphone/core/App.hpp @@ -23,9 +23,10 @@ #include #include "core/singleapplication/singleapplication.h" -#include "core/thread/Thread.hpp" #include "model/core/CoreModel.hpp" +class Thread; + class App : public SingleApplication { public: App(int &argc, char *argv[]); diff --git a/Linphone/core/CMakeLists.txt b/Linphone/core/CMakeLists.txt index ce27949d1..932fe46be 100644 --- a/Linphone/core/CMakeLists.txt +++ b/Linphone/core/CMakeLists.txt @@ -11,7 +11,6 @@ list(APPEND _LINPHONEAPP_SOURCES core/phone-number/PhoneNumberProxy.cpp core/setting/Settings.cpp - core/thread/Thread.cpp core/proxy/ListProxy.cpp core/proxy/Proxy.cpp diff --git a/Linphone/core/account/Account.cpp b/Linphone/core/account/Account.cpp index 5b822ad12..0bbadc797 100644 --- a/Linphone/core/account/Account.cpp +++ b/Linphone/core/account/Account.cpp @@ -21,8 +21,11 @@ #include "Account.hpp" #include "tool/Utils.hpp" +DEFINE_ABSTRACT_OBJECT(Account) + Account::Account(const std::shared_ptr &account) : QObject(nullptr) { // Should be call from model Thread + mustBeInLinphoneThread(getClassName()); auto address = account->getContactAddress(); mContactAddress = address ? Utils::coreStringToAppString(account->getContactAddress()->asString()) : ""; auto params = account->getParams(); @@ -40,6 +43,7 @@ Account::Account(const std::shared_ptr &account) : QObject(nu } Account::~Account() { + mustBeInMainThread("~" + getClassName()); emit mAccountModel->removeListener(); } diff --git a/Linphone/core/account/Account.hpp b/Linphone/core/account/Account.hpp index ea5141bf0..d02ffc3ab 100644 --- a/Linphone/core/account/Account.hpp +++ b/Linphone/core/account/Account.hpp @@ -27,7 +27,7 @@ #include #include -class Account : public QObject { +class Account : public QObject, public AbstractObject { Q_OBJECT Q_PROPERTY(QString contactAddress READ getContactAddress CONSTANT) @@ -66,6 +66,8 @@ private: QString mPictureUri; LinphoneEnums::RegistrationState mRegistrationState; std::shared_ptr mAccountModel; + + DECLARE_ABSTRACT_OBJECT }; #endif diff --git a/Linphone/core/account/AccountList.cpp b/Linphone/core/account/AccountList.cpp index e6ffc0f7e..4a82d3154 100644 --- a/Linphone/core/account/AccountList.cpp +++ b/Linphone/core/account/AccountList.cpp @@ -26,11 +26,14 @@ // ============================================================================= -AccountList::AccountList(QObject *parent) : ListProxy(parent) { +DEFINE_ABSTRACT_OBJECT(AccountList) +AccountList::AccountList(QObject *parent) : ListProxy(parent) { + mustBeInMainThread(getClassName()); App::postModelAsync([=]() { QList> accounts; // Model thread. + mustBeInLinphoneThread(getClassName()); auto linphoneAccounts = CoreModel::getInstance()->getCore()->getAccountList(); for (auto it : linphoneAccounts) { auto model = QSharedPointer(new Account(it), &QObject::deleteLater); @@ -38,9 +41,13 @@ AccountList::AccountList(QObject *parent) : ListProxy(parent) { accounts.push_back(model); } // Invoke for adding stuffs in caller thread - QMetaObject::invokeMethod(this, [this, accounts]() { add(accounts); }); + QMetaObject::invokeMethod(this, [this, accounts]() { + mustBeInMainThread(getClassName()); + add(accounts); + }); }); } AccountList::~AccountList() { + mustBeInMainThread("~" + getClassName()); } diff --git a/Linphone/core/account/AccountList.hpp b/Linphone/core/account/AccountList.hpp index e6aefd229..38b5d8b09 100644 --- a/Linphone/core/account/AccountList.hpp +++ b/Linphone/core/account/AccountList.hpp @@ -22,14 +22,17 @@ #define ACCOUNT_LIST_H_ #include "../proxy/ListProxy.hpp" +#include "tool/AbstractObject.hpp" #include // ============================================================================= -class AccountList : public ListProxy { +class AccountList : public ListProxy, public AbstractObject { Q_OBJECT public: AccountList(QObject *parent = Q_NULLPTR); ~AccountList(); + + DECLARE_ABSTRACT_OBJECT }; #endif diff --git a/Linphone/core/phone-number/PhoneNumber.cpp b/Linphone/core/phone-number/PhoneNumber.cpp index 7d18f1596..d555b778f 100644 --- a/Linphone/core/phone-number/PhoneNumber.cpp +++ b/Linphone/core/phone-number/PhoneNumber.cpp @@ -21,9 +21,11 @@ #include "PhoneNumber.hpp" #include "tool/Utils.hpp" #include +DEFINE_ABSTRACT_OBJECT(PhoneNumber) PhoneNumber::PhoneNumber(const std::shared_ptr &dialPlan) : QObject(nullptr) { // Should be call from model Thread + mustBeInLinphoneThread(getClassName()); mFlag = Utils::coreStringToAppString(dialPlan->getFlag()); mNationalNumberLength = dialPlan->getNationalNumberLength(); mCountryCallingCode = Utils::coreStringToAppString(dialPlan->getCountryCallingCode()); @@ -31,3 +33,7 @@ PhoneNumber::PhoneNumber(const std::shared_ptr &dialPlan) : mInternationalCallPrefix = Utils::coreStringToAppString(dialPlan->getInternationalCallPrefix()); mCountry = Utils::coreStringToAppString(dialPlan->getCountry()); } + +PhoneNumber::~PhoneNumber() { + mustBeInMainThread(getClassName()); +} diff --git a/Linphone/core/phone-number/PhoneNumber.hpp b/Linphone/core/phone-number/PhoneNumber.hpp index 176a2fb94..4a74bd0bd 100644 --- a/Linphone/core/phone-number/PhoneNumber.hpp +++ b/Linphone/core/phone-number/PhoneNumber.hpp @@ -21,10 +21,11 @@ #ifndef PHONE_NUMBER_H_ #define PHONE_NUMBER_H_ +#include "tool/AbstractObject.hpp" #include #include -class PhoneNumber : public QObject { +class PhoneNumber : public QObject, public AbstractObject { Q_OBJECT Q_PROPERTY(QString flag MEMBER mFlag CONSTANT) @@ -37,6 +38,7 @@ class PhoneNumber : public QObject { public: // Should be call from model Thread. Will be automatically in App thread after initialization PhoneNumber(const std::shared_ptr &dialPlan); + ~PhoneNumber(); QString mFlag; int mNationalNumberLength; @@ -44,6 +46,8 @@ public: QString mIsoCountryCode; QString mInternationalCallPrefix; QString mCountry; + + DECLARE_ABSTRACT_OBJECT }; #endif diff --git a/Linphone/core/phone-number/PhoneNumberList.cpp b/Linphone/core/phone-number/PhoneNumberList.cpp index 966177bd2..7e89ff873 100644 --- a/Linphone/core/phone-number/PhoneNumberList.cpp +++ b/Linphone/core/phone-number/PhoneNumberList.cpp @@ -22,12 +22,14 @@ #include "PhoneNumber.hpp" #include "core/App.hpp" #include +#include #include - // ============================================================================= -PhoneNumberList::PhoneNumberList(QObject *parent) : ListProxy(parent) { +DEFINE_ABSTRACT_OBJECT(PhoneNumberList) +PhoneNumberList::PhoneNumberList(QObject *parent) : ListProxy(parent) { + mustBeInMainThread(getClassName()); App::postModelAsync([=]() { // Model thread. auto dialPlans = linphone::Factory::get()->getDialPlans(); @@ -39,6 +41,13 @@ PhoneNumberList::PhoneNumberList(QObject *parent) : ListProxy(parent) { numbers.push_back(numberModel); } // Invoke for adding stuffs in caller thread - QMetaObject::invokeMethod(this, [this, numbers]() { add(numbers); }); + QMetaObject::invokeMethod(this, [this, numbers]() { + mustBeInMainThread(this->log().arg(Q_FUNC_INFO)); + add(numbers); + }); }); } + +PhoneNumberList::~PhoneNumberList() { + mustBeInMainThread("~" + getClassName()); +} diff --git a/Linphone/core/phone-number/PhoneNumberList.hpp b/Linphone/core/phone-number/PhoneNumberList.hpp index 786c9416a..91f8436d3 100644 --- a/Linphone/core/phone-number/PhoneNumberList.hpp +++ b/Linphone/core/phone-number/PhoneNumberList.hpp @@ -22,13 +22,17 @@ #define PHONE_NUMBER_LIST_H_ #include "../proxy/ListProxy.hpp" +#include "tool/AbstractObject.hpp" #include // ============================================================================= -class PhoneNumberList : public ListProxy { +class PhoneNumberList : public ListProxy, public AbstractObject { Q_OBJECT public: PhoneNumberList(QObject *parent = Q_NULLPTR); + ~PhoneNumberList(); + + DECLARE_ABSTRACT_OBJECT }; #endif diff --git a/Linphone/model/account/AccountManager.cpp b/Linphone/model/account/AccountManager.cpp index dcbb045d9..67900326b 100644 --- a/Linphone/model/account/AccountManager.cpp +++ b/Linphone/model/account/AccountManager.cpp @@ -27,13 +27,21 @@ #include "model/core/CoreModel.hpp" #include "tool/Utils.hpp" +DEFINE_ABSTRACT_OBJECT(AccountManager) + AccountManager::AccountManager(QObject *parent) : QObject(parent) { + mustBeInLinphoneThread(getClassName()); +} + +AccountManager::~AccountManager() { + mustBeInLinphoneThread("~" + getClassName()); } std::shared_ptr AccountManager::createAccount(const QString &assistantFile) { + mustBeInLinphoneThread(log().arg(Q_FUNC_INFO)); auto core = CoreModel::getInstance()->getCore(); QString assistantPath = "://data/assistant/" + assistantFile; - qInfo() << QStringLiteral("[AccountManager] Set config on assistant: `%1`.").arg(assistantPath); + qInfo() << log().arg(QStringLiteral("Set config on assistant: `%1`.")).arg(assistantPath); QFile resource(assistantPath); auto file = QTemporaryFile::createNativeFile(resource); core->getConfig()->loadFromXmlFile(Utils::appStringToCoreString(file->fileName())); @@ -41,6 +49,7 @@ std::shared_ptr AccountManager::createAccount(const QString & } bool AccountManager::login(QString username, QString password) { + mustBeInLinphoneThread(log().arg(Q_FUNC_INFO)); auto core = CoreModel::getInstance()->getCore(); auto factory = linphone::Factory::get(); auto account = createAccount("use-app-sip-account.rc"); @@ -51,7 +60,8 @@ bool AccountManager::login(QString username, QString password) { identity->setUsername(Utils::appStringToCoreString(username)); if (params->setIdentityAddress(identity)) { - qWarning() << QStringLiteral("[AccountManager] Unable to set identity address: `%1`.") + qWarning() << log() + .arg(QStringLiteral("Unable to set identity address: `%1`.")) .arg(Utils::coreStringToAppString(identity->asStringUriOnly())); return false; } diff --git a/Linphone/model/account/AccountManager.hpp b/Linphone/model/account/AccountManager.hpp index 35f49144e..2184127c5 100644 --- a/Linphone/model/account/AccountManager.hpp +++ b/Linphone/model/account/AccountManager.hpp @@ -25,11 +25,13 @@ #include #include "AccountModel.hpp" +#include "tool/AbstractObject.hpp" -class AccountManager : public QObject { +class AccountManager : public QObject, public AbstractObject { Q_OBJECT public: AccountManager(QObject *parent = nullptr); + ~AccountManager(); bool login(QString username, QString password); @@ -43,6 +45,7 @@ signals: private: std::shared_ptr mAccountModel; + DECLARE_ABSTRACT_OBJECT }; #endif diff --git a/Linphone/model/account/AccountModel.cpp b/Linphone/model/account/AccountModel.cpp index 43f629986..508add79e 100644 --- a/Linphone/model/account/AccountModel.cpp +++ b/Linphone/model/account/AccountModel.cpp @@ -22,11 +22,15 @@ #include +DEFINE_ABSTRACT_OBJECT(AccountModel) + AccountModel::AccountModel(const std::shared_ptr &account, QObject *parent) : ::Listener(account, parent) { + mustBeInLinphoneThread(getClassName()); } AccountModel::~AccountModel() { + mustBeInLinphoneThread("~" + getClassName()); } void AccountModel::onRegistrationStateChanged(const std::shared_ptr &account, @@ -36,6 +40,7 @@ void AccountModel::onRegistrationStateChanged(const std::shared_ptr(mMonitor); auto params = account->getParams()->clone(); params->setPictureUri(uri); diff --git a/Linphone/model/account/AccountModel.hpp b/Linphone/model/account/AccountModel.hpp index 8aa8f0a94..17f1c5fc6 100644 --- a/Linphone/model/account/AccountModel.hpp +++ b/Linphone/model/account/AccountModel.hpp @@ -22,11 +22,14 @@ #define ACCOUNT_MODEL_H_ #include "model/listener/Listener.hpp" +#include "tool/AbstractObject.hpp" #include #include -class AccountModel : public ::Listener, public linphone::AccountListener { +class AccountModel : public ::Listener, + public linphone::AccountListener, + public AbstractObject { Q_OBJECT public: AccountModel(const std::shared_ptr &account, QObject *parent = nullptr); @@ -44,6 +47,9 @@ signals: const std::string &message); void pictureUriChanged(std::string uri); + +private: + DECLARE_ABSTRACT_OBJECT }; #endif diff --git a/Linphone/model/setting/SettingsModel.cpp b/Linphone/model/setting/SettingsModel.cpp index 059ef9fff..0436d2767 100644 --- a/Linphone/model/setting/SettingsModel.cpp +++ b/Linphone/model/setting/SettingsModel.cpp @@ -19,15 +19,18 @@ */ #include "SettingsModel.hpp" - // ============================================================================= +DEFINE_ABSTRACT_OBJECT(SettingsModel) + const std::string SettingsModel::UiSection("ui"); SettingsModel::SettingsModel(QObject *parent) : QObject(parent) { + mustBeInLinphoneThread(getClassName()); } SettingsModel::~SettingsModel() { + mustBeInLinphoneThread("~" + getClassName()); } bool SettingsModel::isReadOnly(const std::string §ion, const std::string &name) const { @@ -35,5 +38,6 @@ bool SettingsModel::isReadOnly(const std::string §ion, const std::string &na } std::string SettingsModel::getEntryFullName(const std::string §ion, const std::string &name) const { + mustBeInLinphoneThread(log().arg(Q_FUNC_INFO)); return isReadOnly(section, name) ? name + "/readonly" : name; } diff --git a/Linphone/model/setting/SettingsModel.hpp b/Linphone/model/setting/SettingsModel.hpp index 3d7ef1ac3..2b1543407 100644 --- a/Linphone/model/setting/SettingsModel.hpp +++ b/Linphone/model/setting/SettingsModel.hpp @@ -26,7 +26,9 @@ #include #include -class SettingsModel : public QObject { +#include "tool/AbstractObject.hpp" + +class SettingsModel : public QObject, public AbstractObject { Q_OBJECT public: SettingsModel(QObject *parent = Q_NULLPTR); @@ -40,5 +42,8 @@ public: static const std::string UiSection; std::shared_ptr mConfig; + +private: + DECLARE_ABSTRACT_OBJECT }; #endif // SETTINGS_MODEL_H_ diff --git a/Linphone/tool/AbstractObject.hpp b/Linphone/tool/AbstractObject.hpp new file mode 100644 index 000000000..a97707fc5 --- /dev/null +++ b/Linphone/tool/AbstractObject.hpp @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2010-2024 Belledonne Communications SARL. + * + * This file is part of linphone-desktop + * (see https://www.linphone.org). + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef ABSTRACT_OBJECT_H_ +#define ABSTRACT_OBJECT_H_ +#include + +#include "thread/Thread.hpp" + +#define DECLARE_ABSTRACT_OBJECT \ + virtual QString getClassName() const override; \ + static const char *gClassName; + +#define DEFINE_ABSTRACT_OBJECT(CLASS_NAME) \ + const char *CLASS_NAME::gClassName = #CLASS_NAME; \ + QString CLASS_NAME::getClassName() const { \ + return gClassName; \ + } + +class AbstractObject { +public: + virtual QString getClassName() const = 0; + // return "[ClassName]: %1" + inline QString log() const { + return QStringLiteral("[%1]: %2").arg(getClassName()).arg("%1"); + } + + inline static bool mustBeInLinphoneThread(const QString &context) { // For convenience : Alias to Thread + return Thread::mustBeInLinphoneThread(context); + } + inline static bool mustBeInMainThread(const QString &context) { // For convenience : Alias to Thread + return Thread::mustBeInMainThread(context); + } +}; +#endif diff --git a/Linphone/tool/CMakeLists.txt b/Linphone/tool/CMakeLists.txt index 3290f9cbd..9e5064217 100644 --- a/Linphone/tool/CMakeLists.txt +++ b/Linphone/tool/CMakeLists.txt @@ -2,7 +2,7 @@ list(APPEND _LINPHONEAPP_SOURCES tool/Constants.cpp tool/Utils.cpp tool/LinphoneEnums.cpp - + tool/thread/Thread.cpp tool/providers/ImageProvider.cpp ) diff --git a/Linphone/core/thread/Thread.cpp b/Linphone/tool/thread/Thread.cpp similarity index 61% rename from Linphone/core/thread/Thread.cpp rename to Linphone/tool/thread/Thread.cpp index c58a9db1b..41a596d63 100644 --- a/Linphone/core/thread/Thread.cpp +++ b/Linphone/tool/thread/Thread.cpp @@ -19,6 +19,9 @@ */ #include "Thread.hpp" +#include "core/App.hpp" +#include "model/core/CoreModel.hpp" +#include Thread::Thread(QObject *parent) : QThread(parent) { } @@ -30,3 +33,14 @@ void Thread::run() { if (result <= 0) toExit = true; } } + +bool Thread::mustBeInLinphoneThread(const QString &context) { + bool isLinphoneThread = QThread::currentThread() == CoreModel::getInstance()->thread(); + if (!isLinphoneThread) qCritical() << "[Thread] Not processing in Linphone thread from " << context; + return isLinphoneThread; +} +bool Thread::mustBeInMainThread(const QString &context) { + bool isMainThread = QThread::currentThread() == App::getInstance()->thread(); + if (!isMainThread) qCritical() << "[Thread] Not processing in Main thread from " << context; + return isMainThread; +} diff --git a/Linphone/core/thread/Thread.hpp b/Linphone/tool/thread/Thread.hpp similarity index 84% rename from Linphone/core/thread/Thread.hpp rename to Linphone/tool/thread/Thread.hpp index 662543cac..d816e8e75 100644 --- a/Linphone/core/thread/Thread.hpp +++ b/Linphone/tool/thread/Thread.hpp @@ -18,11 +18,17 @@ * along with this program. If not, see . */ +#ifndef THREAD_H_ +#define THREAD_H_ + #include class Thread : public QThread { public: Thread(QObject *parent = nullptr); + static bool mustBeInLinphoneThread(const QString &context); + static bool mustBeInMainThread(const QString &context); virtual void run(); -}; \ No newline at end of file +}; +#endif