From 85e2f79e9eb4b55ba57abf1df9315e7e4bebde84 Mon Sep 17 00:00:00 2001
From: Mate Barany <mate.barany@qt.io>
Date: Thu, 3 Aug 2023 11:17:19 +0200
Subject: [PATCH] Make QNetworkInformation thread safe
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The current implementation is not thread safe - it is possible
to update and query the network information without any locking.

Protect the internals of the backend with a lock and move the
object to the main thread.

Task-number: QTBUG-115748
Change-Id: I9511fc3522ae82cffca42abc54eb79cb156397c4
Reviewed-by: Juha Vuolle <juha.vuolle@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
---
 src/network/kernel/qnetworkinformation.cpp | 13 +++++---
 src/network/kernel/qnetworkinformation_p.h | 37 +++++++++++++++++++---
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/src/network/kernel/qnetworkinformation.cpp b/src/network/kernel/qnetworkinformation.cpp
index fc40787067..aa07fa0a3e 100644
--- a/src/network/kernel/qnetworkinformation.cpp
+++ b/src/network/kernel/qnetworkinformation.cpp
@@ -47,11 +47,6 @@ static void networkInfoCleanup()
     if (!instance)
         return;
 
-    auto needsReinvoke = instance->thread() && instance->thread() != QThread::currentThread();
-    if (needsReinvoke) {
-        QMetaObject::invokeMethod(dataHolder->instanceHolder.get(), []() { networkInfoCleanup(); });
-        return;
-    }
     dataHolder->instanceHolder.reset();
 }
 
@@ -492,6 +487,14 @@ QNetworkInformation::QNetworkInformation(QNetworkInformationBackend *backend)
             &QNetworkInformation::transportMediumChanged);
     connect(backend, &QNetworkInformationBackend::isMeteredChanged, this,
            &QNetworkInformation::isMeteredChanged);
+
+    QThread *main = nullptr;
+
+    if (QCoreApplication::instance())
+        main = QCoreApplication::instance()->thread();
+
+    if (main && thread() != main)
+        moveToThread(main);
 }
 
 /*!
diff --git a/src/network/kernel/qnetworkinformation_p.h b/src/network/kernel/qnetworkinformation_p.h
index 40c12391c5..504955a6e1 100644
--- a/src/network/kernel/qnetworkinformation_p.h
+++ b/src/network/kernel/qnetworkinformation_p.h
@@ -20,6 +20,7 @@
 #include <QtNetwork/qnetworkinformation.h>
 
 #include <QtCore/qloggingcategory.h>
+#include <QtCore/qreadwritelock.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -48,10 +49,29 @@ public:
     virtual QString name() const = 0;
     virtual QNetworkInformation::Features featuresSupported() const = 0;
 
-    Reachability reachability() const { return m_reachability; }
-    bool behindCaptivePortal() const { return m_behindCaptivePortal; }
-    TransportMedium transportMedium() const { return m_transportMedium; }
-    bool isMetered() const { return m_metered; }
+    Reachability reachability() const
+    {
+        QReadLocker locker(&m_lock);
+        return m_reachability;
+    }
+
+    bool behindCaptivePortal() const
+    {
+        QReadLocker locker(&m_lock);
+        return m_behindCaptivePortal;
+    }
+
+    TransportMedium transportMedium() const
+    {
+        QReadLocker locker(&m_lock);
+        return m_transportMedium;
+    }
+
+    bool isMetered() const
+    {
+        QReadLocker locker(&m_lock);
+        return m_metered;
+    }
 
 Q_SIGNALS:
     void reachabilityChanged(Reachability reachability);
@@ -62,37 +82,46 @@ Q_SIGNALS:
 protected:
     void setReachability(QNetworkInformation::Reachability reachability)
     {
+        QWriteLocker locker(&m_lock);
         if (m_reachability != reachability) {
             m_reachability = reachability;
+            locker.unlock();
             emit reachabilityChanged(reachability);
         }
     }
 
     void setBehindCaptivePortal(bool behindPortal)
     {
+        QWriteLocker locker(&m_lock);
         if (m_behindCaptivePortal != behindPortal) {
             m_behindCaptivePortal = behindPortal;
+            locker.unlock();
             emit behindCaptivePortalChanged(behindPortal);
         }
     }
 
     void setTransportMedium(TransportMedium medium)
     {
+        QWriteLocker locker(&m_lock);
         if (m_transportMedium != medium) {
             m_transportMedium = medium;
+            locker.unlock();
             emit transportMediumChanged(medium);
         }
     }
 
     void setMetered(bool isMetered)
     {
+        QWriteLocker locker(&m_lock);
         if (m_metered != isMetered) {
             m_metered = isMetered;
+            locker.unlock();
             emit isMeteredChanged(isMetered);
         }
     }
 
 private:
+    mutable QReadWriteLock m_lock;
     Reachability m_reachability = Reachability::Unknown;
     TransportMedium m_transportMedium = TransportMedium::Unknown;
     bool m_behindCaptivePortal = false;