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: Janne Juntunen <janne.juntunen@qt.io>
Reviewed-by: Samuli Piippo <samuli.piippo@qt.io>

2 files changed, 63 insertions, 22 deletions

diff --git a/src/networksettings/connman/qnetworksettingsmanager_p.cpp b/src/networksettings/connman/qnetworksettingsmanager_p.cpp
index dfa33c4..18a3525 100644
--- a/src/networksettings/connman/qnetworksettingsmanager_p.cpp
+++ b/src/networksettings/connman/qnetworksettingsmanager_p.cpp
@@ -141,8 +141,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();
         }
@@ -152,6 +151,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) {
@@ -234,24 +253,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;
             }
         }
@@ -287,9 +312,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);
     }
 }
 
@@ -304,11 +340,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 c6f31e6..16cb899 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"
@@ -70,10 +71,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;
     QString usbEthernetInternetProtocolAddress();
     QString usbVirtualEthernetLinkProtocol();
     bool hasUsbEthernetProtocolConfiguration();
@@ -99,6 +100,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;
@@ -107,8 +109,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;
 };