Do not leak QNetworkSettingsService instances for unnamed networks

For unnamed/hidden networks, a QNetworkSettingsService object was allocated
every time connman reported these networks via its ServicesChanged signal.
Depending on the system setup, this could happen as often as every 30sec.

We now track these objects in m_unnamedServices and m_unknownServices to
avoid those re-allocations.

Also fixed a potential dangling pointer being returned from
current{Wifi,Wired}Connection()

Fixes: QTBUG-94864
Change-Id: I0a217b1bd334073cc6621397ad09ce6a98776ab1
Reviewed-by: Pasi Petäjäjärvi <pasi.petajajarvi@qt.io>
(cherry picked from commit b80655d9ec266155e3719d91c17ed69e81a3a103)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

2 files changed, 63 insertions, 22 deletions

diff --git a/src/networksettings/connman/qnetworksettingsmanager_p.cpp b/src/networksettings/connman/qnetworksettingsmanager_p.cpp
index 3726a2c..865cab6 100644
--- a/src/networksettings/connman/qnetworksettingsmanager_p.cpp
+++ b/src/networksettings/connman/qnetworksettingsmanager_p.cpp
@@ -140,8 +140,7 @@ void QNetworkSettingsManagerPrivate::tryNextConnection()
     }
     if (!service) {
         if (!m_unnamedServicesForSsidConnection.isEmpty()) {
-            service = *m_unnamedServicesForSsidConnection.begin();
-            m_unnamedServicesForSsidConnection.erase(m_unnamedServicesForSsidConnection.begin());
+            service = m_unnamedServicesForSsidConnection.take(m_unnamedServicesForSsidConnection.firstKey());
         } else {
             q->clearConnectionState();
         }
@@ -151,6 +150,26 @@ void QNetworkSettingsManagerPrivate::tryNextConnection()
     }
 }
 
+void QNetworkSettingsManagerPrivate::setCurrentWifiConnection(QNetworkSettingsService *connection)
+{
+    m_currentWifiConnection = connection;
+}
+
+QNetworkSettingsService *QNetworkSettingsManagerPrivate::currentWifiConnection() const
+{
+    return m_currentWifiConnection.data();
+}
+
+void QNetworkSettingsManagerPrivate::setCurrentWiredConnection(QNetworkSettingsService *connection)
+{
+    m_currentWiredConnection = connection;
+}
+
+QNetworkSettingsService *QNetworkSettingsManagerPrivate::currentWiredConnection() const
+{
+    return m_currentWiredConnection.data();
+}
+
 void QNetworkSettingsManagerPrivate::onConnmanServiceRegistered(const QString &serviceName)
 {
     if (serviceName == ConnManServiceName) {
@@ -233,24 +252,30 @@ void QNetworkSettingsManagerPrivate::getTechnologiesFinished(QDBusPendingCallWat
 void QNetworkSettingsManagerPrivate::onServicesChanged(ConnmanMapStructList changed, const QList<QDBusObjectPath> &removed)
 {
     Q_Q(QNetworkSettingsManager);
+    foreach (const QDBusObjectPath &dpath, removed) {
+        QString path = dpath.path();
 
-    foreach (QDBusObjectPath path, removed) {
-        if (m_serviceModel->removeService(path.path()))
+        if (m_serviceModel->removeService(path))
             emit q->servicesChanged();
 
-        auto serviceIter = m_unnamedServices.find(path.path());
-        if (serviceIter != m_unnamedServices.end()) {
-            serviceIter.value()->deleteLater();
-            m_unnamedServices.erase(serviceIter);
-        }
+        if (auto service = m_unnamedServices.take(path))
+            service->deleteLater();
+        if (auto service = m_unknownServices.take(path))
+            service->deleteLater();
+        m_unnamedServicesForSsidConnection.remove(path); // do not delete here
     }
 
     QStringList newServices;
-    foreach (ConnmanMapStruct map, changed) {
+    foreach (const ConnmanMapStruct &map, changed) {
+        QString path = map.objectPath.path();
+
+        if (m_unknownServices.contains(path) || m_unnamedServices.contains(path))
+            continue;
+
         bool found = false;
         foreach (QNetworkSettingsService* service, m_serviceModel->getModel()) {
-            if (service->id() == map.objectPath.path() && service->placeholderState() == false) {
-                found =true;
+            if (service->id() == path && service->placeholderState() == false) {
+                found = true;
                 break;
             }
         }
@@ -286,9 +311,20 @@ void QNetworkSettingsManagerPrivate::handleNewService(const QString &servicePath
         }
     }
     else {
+        bool isUnnamedWifi = false;
+
         //Service name or type not set, wait for update
-        connect(service, &QNetworkSettingsService::nameChanged, this, &QNetworkSettingsManagerPrivate::serviceReady);
-        connect(service, &QNetworkSettingsService::typeChanged, this, &QNetworkSettingsManagerPrivate::serviceReady);
+        if (service->name().isEmpty()) {
+            connect(service, &QNetworkSettingsService::nameChanged, this, &QNetworkSettingsManagerPrivate::serviceReady);
+            isUnnamedWifi = (service->type() == QNetworkSettingsType::Wifi);
+        }
+        if (service->type() == QNetworkSettingsType::Unknown)
+            connect(service, &QNetworkSettingsService::typeChanged, this, &QNetworkSettingsManagerPrivate::serviceReady);
+
+        if (isUnnamedWifi)
+            m_unnamedServices.insert(service->id(), service);
+        else
+            m_unknownServices.insert(service->id(), service);
     }
 }
 
@@ -303,11 +339,14 @@ void QNetworkSettingsManagerPrivate::serviceReady()
 
     QNetworkSettingsService* service = qobject_cast<QNetworkSettingsService*>(sender());
 
-    if (service->type() != QNetworkSettingsType::Unknown
-            && service->type() == QNetworkSettingsType::Wifi) {
+    // the type changed from Unknown to Wifi
+    if ((service->type() == QNetworkSettingsType::Wifi)
+            && m_unknownServices.contains(service->id())) {
+        m_unknownServices.remove(service->id());
         m_unnamedServices.insert(service->id(), service);
     }
 
+    // we have a name and a length now
     if (service->name().length() > 0 && service->type() != QNetworkSettingsType::Unknown) {
         service->disconnect(this);
         m_unnamedServices.remove(service->id());
diff --git a/src/networksettings/connman/qnetworksettingsmanager_p.h b/src/networksettings/connman/qnetworksettingsmanager_p.h
index c696cdd..7d0cd54 100644
--- a/src/networksettings/connman/qnetworksettingsmanager_p.h
+++ b/src/networksettings/connman/qnetworksettingsmanager_p.h
@@ -43,6 +43,7 @@
 #include <QObject>
 #include <QtDBus>
 #include <QMap>
+#include <QPointer>
 #include "connmancommon.h"
 #include "qnetworksettingsmanager.h"
 #include "qnetworksettingsinterfacemodel.h"
@@ -69,10 +70,10 @@ public:
     void connectBySsid(const QString &name);
     void clearConnectionState();
     void tryNextConnection();
-    void setCurrentWifiConnection(QNetworkSettingsService *connection) {m_currentWifiConnection = connection;}
-    QNetworkSettingsService* currentWifiConnection() const {return m_currentWifiConnection;}
-    void setCurrentWiredConnection(QNetworkSettingsService *connection) {m_currentWiredConnection = connection;}
-    QNetworkSettingsService* currentWiredConnection() const {return m_currentWiredConnection;}
+    void setCurrentWifiConnection(QNetworkSettingsService *connection);
+    QNetworkSettingsService* currentWifiConnection() const;
+    void setCurrentWiredConnection(QNetworkSettingsService *connection);
+    QNetworkSettingsService* currentWiredConnection() const;
 
 public slots:
     void getServicesFinished(QDBusPendingCallWatcher *watcher);
@@ -91,6 +92,7 @@ private:
 protected:
     QNetworkSettingsInterfaceModel m_interfaceModel;
     QNetworkSettingsServiceModel *m_serviceModel;
+    QMap<QString, QNetworkSettingsService*> m_unknownServices;
     QMap<QString, QNetworkSettingsService*> m_unnamedServices;
     QMap<QString, QNetworkSettingsService*> m_unnamedServicesForSsidConnection;
     QNetworkSettingsServiceFilter *m_serviceFilter;
@@ -99,8 +101,8 @@ private:
     QNetworkSettingsUserAgent *m_agent;
     QDBusServiceWatcher *m_serviceWatcher;
     QString m_currentSsid;
-    QNetworkSettingsService *m_currentWifiConnection;
-    QNetworkSettingsService *m_currentWiredConnection;
+    QPointer<QNetworkSettingsService> m_currentWifiConnection;
+    QPointer<QNetworkSettingsService> m_currentWiredConnection;
     bool m_initialized;
 };