Fix the QNetworkConnectionManagerPrivate initialisation code.

The current code was meant to be a thread-safe initialisation that
also ran a couple of extra steps. But it wasn't. While it's ok to call
qAddPostRoutine(), the call to updateConfigurations() was
thread-unsafe. It is possible that another thread got the pointer to
the Private before updateConfigurations() finished.

So instead protect the initialisation with a mutex.

It's possible that the value of the pointer becomes visible to
other processors before the other contained values, so use
atomics here.

To call qAddPostRoutine safely from the main thread, use the trick
of deleteLater() (which is thread-safe) in another thread connecting
to a slot.

Change-Id: If9bab88138755df95a791f34b0be8684207979d7
Reviewed-on: http://codereview.qt-project.org/5028
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Bradley T. Hughes <bradley.hughes@nokia.com>
Reviewed-by: Lars Knoll <lars.knoll@nokia.com>

2 files changed, 46 insertions, 22 deletions

diff --git a/src/network/bearer/qnetworkconfigmanager.cpp b/src/network/bearer/qnetworkconfigmanager.cpp
index 666aba7e82..6c73828096 100644
--- a/src/network/bearer/qnetworkconfigmanager.cpp
+++ b/src/network/bearer/qnetworkconfigmanager.cpp
@@ -46,34 +46,55 @@
 
 #include <QtCore/qstringlist.h>
 #include <QtCore/qcoreapplication.h>
+#include <QtCore/qmutex.h>
+#include <QtCore/qthread.h>
+#include <QtCore/private/qcoreapplication_p.h>
 
 #ifndef QT_NO_BEARERMANAGEMENT
 
 QT_BEGIN_NAMESPACE
 
-#define Q_GLOBAL_STATIC_QAPP_DESTRUCTION(TYPE, NAME)                    \
-    static QGlobalStatic<TYPE > this_##NAME                             \
-                = { Q_BASIC_ATOMIC_INITIALIZER(0), false };             \
-    static void NAME##_cleanup()                                        \
-    {                                                                   \
-        delete this_##NAME.pointer;                                     \
-        this_##NAME.pointer = 0;                                        \
-    }                                                                   \
-    static TYPE *NAME()                                                 \
-    {                                                                   \
-        if (!this_##NAME.pointer) {                                     \
-            TYPE *x = new TYPE;                                         \
-            if (!this_##NAME.pointer.testAndSetOrdered(0, x))           \
-                delete x;                                               \
-            else {                                                      \
-                qAddPostRoutine(NAME##_cleanup);                        \
-                this_##NAME.pointer->updateConfigurations();            \
-            }                                                           \
-        }                                                               \
-        return this_##NAME.pointer;                                     \
-    }
+static QBasicAtomicPointer<QNetworkConfigurationManagerPrivate> connManager_ptr;
+Q_GLOBAL_STATIC(QMutex, connManager_mutex)
+
+static void connManager_cleanup()
+{
+    // this is not atomic or thread-safe!
+    delete connManager_ptr.load();
+    connManager_ptr.store(0);
+}
+
+void QNetworkConfigurationManagerPrivate::addPostRoutine()
+{
+    qAddPostRoutine(connManager_cleanup);
+}
 
-Q_GLOBAL_STATIC_QAPP_DESTRUCTION(QNetworkConfigurationManagerPrivate, connManager);
+static QNetworkConfigurationManagerPrivate *connManager()
+{
+    QNetworkConfigurationManagerPrivate *ptr = connManager_ptr.loadAcquire();
+    if (!ptr) {
+        QMutexLocker locker(connManager_mutex());
+        if (!(ptr = connManager_ptr.loadAcquire())) {
+            ptr = new QNetworkConfigurationManagerPrivate;
+
+            if (QCoreApplicationPrivate::mainThread() == QThread::currentThread()) {
+                // right thread or no main thread yet
+                ptr->addPostRoutine();
+                ptr->updateConfigurations();
+            } else {
+                // wrong thread, we need to make the main thread do this
+                QObject *obj = new QObject;
+                QObject::connect(obj, SIGNAL(destroyed()), ptr, SLOT(addPostRoutine()), Qt::DirectConnection);
+                ptr->updateConfigurations(); // this moves us to the main thread
+                obj->moveToThread(QCoreApplicationPrivate::mainThread());
+                obj->deleteLater();
+            }
+
+            connManager_ptr.storeRelease(ptr);
+        }
+    }
+    return ptr;
+}
 
 QNetworkConfigurationManagerPrivate *qNetworkConfigurationManagerPrivate()
 {
diff --git a/src/network/bearer/qnetworkconfigmanager_p.h b/src/network/bearer/qnetworkconfigmanager_p.h
index 04cce202c5..e73e9bb399 100644
--- a/src/network/bearer/qnetworkconfigmanager_p.h
+++ b/src/network/bearer/qnetworkconfigmanager_p.h
@@ -92,6 +92,8 @@ public:
 public Q_SLOTS:
     void updateConfigurations();
 
+    static void addPostRoutine();
+
 Q_SIGNALS:
     void configurationAdded(const QNetworkConfiguration &config);
     void configurationRemoved(const QNetworkConfiguration &config);
@@ -106,6 +108,7 @@ private Q_SLOTS:
 
     void pollEngines();
 
+
 private:
     Q_INVOKABLE void startPolling();
     QTimer *pollTimer;