From 5dfaab214d29133bbb2f7e23726f2ef8cd8e8e08 Mon Sep 17 00:00:00 2001 From: Robert Griebl Date: Thu, 19 Aug 2021 16:42:41 +0200 Subject: Do not leak QNetworkSettingsService instances for unnamed networks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 (cherry picked from commit b80655d9ec266155e3719d91c17ed69e81a3a103) Reviewed-by: Janne Juntunen Reviewed-by: Samuli Piippo --- .../connman/qnetworksettingsmanager_p.cpp | 71 +++++++++++++++++----- .../connman/qnetworksettingsmanager_p.h | 14 +++-- 2 files changed, 63 insertions(+), 22 deletions(-) (limited to 'src/networksettings') 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 &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(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 #include #include +#include #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 m_unknownServices; QMap m_unnamedServices; QMap 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 m_currentWifiConnection; + QPointer m_currentWiredConnection; bool m_initialized; }; -- cgit v1.2.3