fix: change event looping to mutexes for settings and secureqsettings (#2270)

client/secure_qsettings.cpp

@@ -35,13 +35,12 @@ SecureQSettings::SecureQSettings(const QString &organization, const QString &app
             }
         }
         m_settings.setValue("Conf/encrypted", true);
-        m_settings.sync();
     }
 }
 
 QVariant SecureQSettings::value(const QString &key, const QVariant &defaultValue) const
 {
-    QMutexLocker locker(&mutex);
+    QMutexLocker locker(&m_mutex);
 
     if (m_cache.contains(key)) {
         return m_cache.value(key);
@@ -85,7 +84,7 @@ QVariant SecureQSettings::value(const QString &key, const QVariant &defaultValue
 
 void SecureQSettings::setValue(const QString &key, const QVariant &value)
 {
-    QMutexLocker locker(&mutex);
+    QMutexLocker locker(&m_mutex);
 
     if (encryptionRequired() && encryptedKeys.contains(key)) {
         if (!getEncKey().isEmpty() && !getEncIv().isEmpty()) {
@@ -107,26 +106,20 @@ void SecureQSettings::setValue(const QString &key, const QVariant &value)
     }
 
     m_cache.insert(key, value);
-    sync();
 }
 
 void SecureQSettings::remove(const QString &key)
 {
-    QMutexLocker locker(&mutex);
+    QMutexLocker locker(&m_mutex);
 
     m_settings.remove(key);
     m_cache.remove(key);
-
-    sync();
-}
-
-void SecureQSettings::sync()
-{
-    m_settings.sync();
 }
 
 QByteArray SecureQSettings::backupAppConfig() const
 {
+    QMutexLocker locker(&m_mutex);
+
     QJsonObject cfg;
 
     const auto needToBackup = [this](const auto &key) {
@@ -161,6 +154,8 @@ QByteArray SecureQSettings::backupAppConfig() const
 
 bool SecureQSettings::restoreAppConfig(const QByteArray &json)
 {
+    QMutexLocker locker(&m_mutex);
+
     QJsonObject cfg = QJsonDocument::fromJson(json).object();
     if (cfg.isEmpty())
         return false;
@@ -173,10 +168,16 @@ bool SecureQSettings::restoreAppConfig(const QByteArray &json)
         setValue(key, cfg.value(key).toVariant());
     }
 
-    sync();
     return true;
 }
 
+void SecureQSettings::clearSettings()
+{
+    QMutexLocker locker(&m_mutex);
+    m_settings.clear();
+    m_cache.clear();
+}
+
 QByteArray SecureQSettings::encryptText(const QByteArray &value) const
 {
     QSimpleCrypto::QBlockCipher cipher;
@@ -294,11 +295,3 @@ void SecureQSettings::setSecTag(const QString &tag, const QByteArray &data)
         qCritical() << "SecureQSettings::setSecTag Error:" << job->errorString();
     }
 }
-
-void SecureQSettings::clearSettings()
-{
-    QMutexLocker locker(&mutex);
-    m_settings.clear();
-    m_cache.clear();
-    sync();
-}

client/secure_qsettings.h

@@ -16,14 +16,16 @@ public:
     explicit SecureQSettings(const QString &organization, const QString &application = QString(),
                              QObject *parent = nullptr);
 
-    Q_INVOKABLE QVariant value(const QString &key, const QVariant &defaultValue = QVariant()) const;
+    QVariant value(const QString &key, const QVariant &defaultValue = QVariant()) const;
-    Q_INVOKABLE void setValue(const QString &key, const QVariant &value);
+    void setValue(const QString &key, const QVariant &value);
     void remove(const QString &key);
-    void sync();
 
     QByteArray backupAppConfig() const;
     bool restoreAppConfig(const QByteArray &json);
 
+    void clearSettings();
+
+private:
     QByteArray encryptText(const QByteArray &value) const;
     QByteArray decryptText(const QByteArray &ba) const;
 
@@ -35,9 +37,6 @@ public:
     static QByteArray getSecTag(const QString &tag);
     static void setSecTag(const QString &tag, const QByteArray &data);
 
-    void clearSettings();
-
-private:
     QSettings m_settings;
 
     mutable QHash<QString, QVariant> m_cache;
@@ -53,7 +52,7 @@ private:
 
     const QByteArray magicString { "EncData" }; // Magic keyword used for mark encrypted QByteArray
 
-    mutable QMutex mutex;
+    mutable QRecursiveMutex m_mutex;
 };
 
 #endif // SECUREQSETTINGS_H

client/settings.cpp

@@ -21,10 +21,10 @@ Settings::Settings(QObject *parent) : QObject(parent), m_settings(ORGANIZATION_N
 {
     // Import old settings
     if (serversCount() == 0) {
-        QString user = value("Server/userName").toString();
+        QString user = m_settings.value("Server/userName").toString();
-        QString password = value("Server/password").toString();
+        QString password = m_settings.value("Server/password").toString();
-        QString serverName = value("Server/serverName").toString();
+        QString serverName = m_settings.value("Server/serverName").toString();
-        int port = value("Server/serverPort").toInt();
+        int port = m_settings.value("Server/serverPort").toInt();
 
         if (!user.isEmpty() && !password.isEmpty() && !serverName.isEmpty()) {
             QJsonObject server;
@@ -222,7 +222,7 @@ QString Settings::nextAvailableServerName() const
 
 void Settings::setSaveLogs(bool enabled)
 {
-    setValue("Conf/saveLogs", enabled);
+    m_settings.setValue("Conf/saveLogs", enabled);
 #ifndef Q_OS_ANDROID
     if (!isSaveLogs()) {
         Logger::deInit();
@@ -242,12 +242,12 @@ void Settings::setSaveLogs(bool enabled)
 
 QDateTime Settings::getLogEnableDate()
 {
-    return value("Conf/logEnableDate").toDateTime();
+    return m_settings.value("Conf/logEnableDate").toDateTime();
 }
 
 void Settings::setLogEnableDate(QDateTime date)
 {
-    setValue("Conf/logEnableDate", date);
+    m_settings.setValue("Conf/logEnableDate", date);
 }
 
 QString Settings::routeModeString(RouteMode mode) const
@@ -261,17 +261,17 @@ QString Settings::routeModeString(RouteMode mode) const
 
 Settings::RouteMode Settings::routeMode() const
 {
-    return static_cast<RouteMode>(value("Conf/routeMode", 0).toInt());
+    return static_cast<RouteMode>(m_settings.value("Conf/routeMode", 0).toInt());
 }
 
 bool Settings::isSitesSplitTunnelingEnabled() const
 {
-    return value("Conf/sitesSplitTunnelingEnabled", false).toBool();
+    return m_settings.value("Conf/sitesSplitTunnelingEnabled", false).toBool();
 }
 
 void Settings::setSitesSplitTunnelingEnabled(bool enabled)
 {
-    setValue("Conf/sitesSplitTunnelingEnabled", enabled);
+    m_settings.setValue("Conf/sitesSplitTunnelingEnabled", enabled);
 }
 
 bool Settings::addVpnSite(RouteMode mode, const QString &site, const QString &ip)
@@ -359,12 +359,12 @@ void Settings::removeAllVpnSites(RouteMode mode)
 
 QString Settings::primaryDns() const
 {
-    return value("Conf/primaryDns", cloudFlareNs1).toString();
+    return m_settings.value("Conf/primaryDns", cloudFlareNs1).toString();
 }
 
 QString Settings::secondaryDns() const
 {
-    return value("Conf/secondaryDns", cloudFlareNs2).toString();
+    return m_settings.value("Conf/secondaryDns", cloudFlareNs2).toString();
 }
 
 void Settings::clearSettings()
@@ -386,18 +386,18 @@ QString Settings::appsRouteModeString(AppsRouteMode mode) const
 
 Settings::AppsRouteMode Settings::getAppsRouteMode() const
 {
-    return static_cast<AppsRouteMode>(value("Conf/appsRouteMode", 0).toInt());
+    return static_cast<AppsRouteMode>(m_settings.value("Conf/appsRouteMode", 0).toInt());
 }
 
 void Settings::setAppsRouteMode(AppsRouteMode mode)
 {
-    setValue("Conf/appsRouteMode", mode);
+    m_settings.setValue("Conf/appsRouteMode", mode);
 }
 
 QVector<InstalledAppInfo> Settings::getVpnApps(AppsRouteMode mode) const
 {
     QVector<InstalledAppInfo> apps;
-    auto appsArray = value("Conf/" + appsRouteModeString(mode)).toJsonArray();
+    auto appsArray = m_settings.value("Conf/" + appsRouteModeString(mode)).toJsonArray();
     for (const auto &app : appsArray) {
         InstalledAppInfo appInfo;
         appInfo.appName = app.toObject().value("appName").toString();
@@ -419,43 +419,42 @@ void Settings::setVpnApps(AppsRouteMode mode, const QVector<InstalledAppInfo> &a
         appInfo.insert("appPath", app.appPath);
         appsArray.push_back(appInfo);
     }
-    setValue("Conf/" + appsRouteModeString(mode), appsArray);
+    m_settings.setValue("Conf/" + appsRouteModeString(mode), appsArray);
-    m_settings.sync();
 }
 
 bool Settings::isAppsSplitTunnelingEnabled() const
 {
-    return value("Conf/appsSplitTunnelingEnabled", false).toBool();
+    return m_settings.value("Conf/appsSplitTunnelingEnabled", false).toBool();
 }
 
 void Settings::setAppsSplitTunnelingEnabled(bool enabled)
 {
-    setValue("Conf/appsSplitTunnelingEnabled", enabled);
+    m_settings.setValue("Conf/appsSplitTunnelingEnabled", enabled);
 }
 
 bool Settings::isKillSwitchEnabled() const
 {
-    return value("Conf/killSwitchEnabled", true).toBool();
+    return m_settings.value("Conf/killSwitchEnabled", true).toBool();
 }
 
 void Settings::setKillSwitchEnabled(bool enabled)
 {
-    setValue("Conf/killSwitchEnabled", enabled);
+    m_settings.setValue("Conf/killSwitchEnabled", enabled);
 }
 
 bool Settings::isStrictKillSwitchEnabled() const
 {
-    return value("Conf/strictKillSwitchEnabled", false).toBool();
+    return m_settings.value("Conf/strictKillSwitchEnabled", false).toBool();
 }
 
 void Settings::setStrictKillSwitchEnabled(bool enabled)
 {
-    setValue("Conf/strictKillSwitchEnabled", enabled);
+    m_settings.setValue("Conf/strictKillSwitchEnabled", enabled);
 }
 
 QString Settings::getInstallationUuid(const bool needCreate)
 {
-    auto uuid = value("Conf/installationUuid", "").toString();
+    auto uuid = m_settings.value("Conf/installationUuid", "").toString();
     if (needCreate && uuid.isEmpty()) {
         uuid = QUuid::createUuid().toString();
 
@@ -476,7 +475,7 @@ QString Settings::getInstallationUuid(const bool needCreate)
 
 void Settings::setInstallationUuid(const QString &uuid)
 {
-    setValue("Conf/installationUuid", uuid);
+    m_settings.setValue("Conf/installationUuid", uuid);
 }
 
 ServerCredentials Settings::defaultServerCredentials() const
@@ -497,28 +496,6 @@ ServerCredentials Settings::serverCredentials(int index) const
     return credentials;
 }
 
-QVariant Settings::value(const QString &key, const QVariant &defaultValue) const
-{
-    QVariant returnValue;
-    if (QThread::currentThread() == QCoreApplication::instance()->thread()) {
-        returnValue = m_settings.value(key, defaultValue);
-    } else {
-        QMetaObject::invokeMethod(&m_settings, "value", Qt::BlockingQueuedConnection, Q_RETURN_ARG(QVariant, returnValue),
-                                  Q_ARG(const QString &, key), Q_ARG(const QVariant &, defaultValue));
-    }
-    return returnValue;
-}
-
-void Settings::setValue(const QString &key, const QVariant &value)
-{
-    if (QThread::currentThread() == QCoreApplication::instance()->thread()) {
-        m_settings.setValue(key, value);
-    } else {
-        QMetaObject::invokeMethod(&m_settings, "setValue", Qt::BlockingQueuedConnection, Q_ARG(const QString &, key),
-                                  Q_ARG(const QVariant &, value));
-    }
-}
-
 void Settings::resetGatewayEndpoint()
 {
     m_gatewayEndpoint = gatewayEndpoint;
@@ -541,50 +518,50 @@ QString Settings::getGatewayEndpoint(bool isTestPurchase)
 
 bool Settings::isDevGatewayEnv(bool isTestPurchase)
 {
-    return isTestPurchase ? true : value("Conf/devGatewayEnv", false).toBool();
+    return isTestPurchase ? true : m_settings.value("Conf/devGatewayEnv", false).toBool();
 }
 
 void Settings::toggleDevGatewayEnv(bool enabled)
 {
-    setValue("Conf/devGatewayEnv", enabled);
+    m_settings.setValue("Conf/devGatewayEnv", enabled);
 }
 
 bool Settings::isHomeAdLabelVisible()
 {
-    return value("Conf/homeAdLabelVisible", true).toBool();
+    return m_settings.value("Conf/homeAdLabelVisible", true).toBool();
 }
 
 void Settings::disableHomeAdLabel()
 {
-    setValue("Conf/homeAdLabelVisible", false);
+    m_settings.setValue("Conf/homeAdLabelVisible", false);
 }
 
 bool Settings::isPremV1MigrationReminderActive()
 {
-    return value("Conf/premV1MigrationReminderActive", true).toBool();
+    return m_settings.value("Conf/premV1MigrationReminderActive", true).toBool();
 }
 
 void Settings::disablePremV1MigrationReminder()
 {
-    setValue("Conf/premV1MigrationReminderActive", false);
+    m_settings.setValue("Conf/premV1MigrationReminderActive", false);
 }
 
 QStringList Settings::allowedDnsServers() const
 {
-    return value("Conf/allowedDnsServers").toStringList();
+    return m_settings.value("Conf/allowedDnsServers").toStringList();
 }
 
 void Settings::setAllowedDnsServers(const QStringList &servers)
 {
-    setValue("Conf/allowedDnsServers", servers);
+    m_settings.setValue("Conf/allowedDnsServers", servers);
 }
 
 QStringList Settings::readNewsIds() const
 {
-    return value("News/readIds").toStringList();
+    return m_settings.value("News/readIds").toStringList();
 }
 
 void Settings::setReadNewsIds(const QStringList &ids)
 {
-    setValue("News/readIds", ids);
+    m_settings.setValue("News/readIds", ids);
 }

client/settings.h

@@ -29,11 +29,11 @@ public:
 
     QJsonArray serversArray() const
     {
-        return QJsonDocument::fromJson(value("Servers/serversList").toByteArray()).array();
+        return QJsonDocument::fromJson(m_settings.value("Servers/serversList").toByteArray()).array();
     }
     void setServersArray(const QJsonArray &servers)
     {
-        setValue("Servers/serversList", QJsonDocument(servers).toJson());
+        m_settings.setValue("Servers/serversList", QJsonDocument(servers).toJson());
     }
 
     // Servers section
@@ -45,11 +45,11 @@ public:
 
     int defaultServerIndex() const
     {
-        return value("Servers/defaultServerIndex", 0).toInt();
+        return m_settings.value("Servers/defaultServerIndex", 0).toInt();
     }
     void setDefaultServer(int index)
     {
-        setValue("Servers/defaultServerIndex", index);
+        m_settings.setValue("Servers/defaultServerIndex", index);
     }
     QJsonObject defaultServer() const
     {
@@ -78,34 +78,34 @@ public:
     // App settings section
     bool isAutoConnect() const
     {
-        return value("Conf/autoConnect", false).toBool();
+        return m_settings.value("Conf/autoConnect", false).toBool();
     }
     void setAutoConnect(bool enabled)
     {
-        setValue("Conf/autoConnect", enabled);
+        m_settings.setValue("Conf/autoConnect", enabled);
     }
 
     bool isStartMinimized() const
     {
-        return value("Conf/startMinimized", false).toBool();
+        return m_settings.value("Conf/startMinimized", false).toBool();
     }
     void setStartMinimized(bool enabled)
     {
-        setValue("Conf/startMinimized", enabled);
+        m_settings.setValue("Conf/startMinimized", enabled);
     }
 
     bool isNewsNotifications() const
     {
-        return value("Conf/newsNotifications", true).toBool();
+        return m_settings.value("Conf/newsNotifications", true).toBool();
     }
     void setNewsNotifications(bool enabled)
     {
-        setValue("Conf/newsNotifications", enabled);
+        m_settings.setValue("Conf/newsNotifications", enabled);
     }
 
     bool isSaveLogs() const
     {
-        return value("Conf/saveLogs", false).toBool();
+        return m_settings.value("Conf/saveLogs", false).toBool();
     }
     void setSaveLogs(bool enabled);
 
@@ -122,19 +122,18 @@ public:
     QString routeModeString(RouteMode mode) const;
 
     RouteMode routeMode() const;
-    void setRouteMode(RouteMode mode) { setValue("Conf/routeMode", mode); }
+    void setRouteMode(RouteMode mode) { m_settings.setValue("Conf/routeMode", mode); }
 
     bool isSitesSplitTunnelingEnabled() const;
     void setSitesSplitTunnelingEnabled(bool enabled);
 
     QVariantMap vpnSites(RouteMode mode) const
     {
-        return value("Conf/" + routeModeString(mode)).toMap();
+        return m_settings.value("Conf/" + routeModeString(mode)).toMap();
     }
     void setVpnSites(RouteMode mode, const QVariantMap &sites)
     {
-        setValue("Conf/" + routeModeString(mode), sites);
+        m_settings.setValue("Conf/" + routeModeString(mode), sites);
-        m_settings.sync();
     }
     bool addVpnSite(RouteMode mode, const QString &site, const QString &ip = "");
     void addVpnSites(RouteMode mode, const QMap<QString, QString> &sites); // map <site, ip>
@@ -147,11 +146,11 @@ public:
 
     bool useAmneziaDns() const
     {
-        return value("Conf/useAmneziaDns", true).toBool();
+        return m_settings.value("Conf/useAmneziaDns", true).toBool();
     }
     void setUseAmneziaDns(bool enabled)
     {
-        setValue("Conf/useAmneziaDns", enabled);
+        m_settings.setValue("Conf/useAmneziaDns", enabled);
     }
 
     QString primaryDns() const;
@@ -160,13 +159,13 @@ public:
     // QString primaryDns() const { return m_primaryDns; }
     void setPrimaryDns(const QString &primaryDns)
     {
-        setValue("Conf/primaryDns", primaryDns);
+        m_settings.setValue("Conf/primaryDns", primaryDns);
     }
 
     // QString secondaryDns() const { return m_secondaryDns; }
     void setSecondaryDns(const QString &secondaryDns)
     {
-        setValue("Conf/secondaryDns", secondaryDns);
+        m_settings.setValue("Conf/secondaryDns", secondaryDns);
     }
 
     //    static constexpr char openNicNs5[] = "94.103.153.176";
@@ -188,16 +187,16 @@ public:
     };
     void setAppLanguage(QLocale locale)
     {
-        setValue("Conf/appLanguage", locale.name());
+        m_settings.setValue("Conf/appLanguage", locale.name());
     };
 
     bool isScreenshotsEnabled() const
     {
-        return value("Conf/screenshotsEnabled", true).toBool();
+        return m_settings.value("Conf/screenshotsEnabled", true).toBool();
     }
     void setScreenshotsEnabled(bool enabled)
     {
-        setValue("Conf/screenshotsEnabled", enabled);
+        m_settings.setValue("Conf/screenshotsEnabled", enabled);
         emit screenshotsEnabledChanged(enabled);
     }
 
@@ -255,9 +254,6 @@ signals:
     void settingsCleared();
 
 private:
-    QVariant value(const QString &key, const QVariant &defaultValue = QVariant()) const;
-    void setValue(const QString &key, const QVariant &value);
-
     void setInstallationUuid(const QString &uuid);
 
     mutable SecureQSettings m_settings;

service/server/killswitch.cpp

@@ -71,7 +71,6 @@ bool KillSwitch::isStrictKillSwitchEnabled()
                              + "\\" + QString(APPLICATION_NAME), QSettings::NativeFormat);
     return RegHLM.value("strictKillSwitchEnabled", false).toBool();
 #endif
-    m_appSettigns->sync();
     return m_appSettigns->value("Conf/strictKillSwitchEnabled", false).toBool();
 }