Make ModelManagerInterface::instance() a bit safer

Currently, in d'tor of ModelManagerInterface, when joinAllThreads()
has finished, we were setting the g_instance to nullptr. However,
just after the joinAllThreads() has finished and before setting
the g_instance to nullptr some other thread could still add a
new future and this thread could potentially still assume that
instance() will return valid pointer.

The fix is to make joinAllThreads() and setting the g_instance to
nullptr in an atomic way. We ensure, that when there are still
pending futures, we don't clear the instance, but call joinAllThreads()
again for them.

The implementation is similar to what we do in test_joinAllThreads().

Change-Id: I99dc341228aee710a958a0fbc6c5ec1fbe132e7f
Reviewed-by: Fawzi Mohamed <fawzi.mohamed@qt.io>

1 files changed, 12 insertions, 4 deletions

diff --git a/src/libs/qmljs/qmljsmodelmanagerinterface.cpp b/src/libs/qmljs/qmljsmodelmanagerinterface.cpp
index 8eae3c3625..f500114016 100644
--- a/src/libs/qmljs/qmljsmodelmanagerinterface.cpp
+++ b/src/libs/qmljs/qmljsmodelmanagerinterface.cpp
@@ -142,13 +142,20 @@ ModelManagerInterface::ModelManagerInterface(QObject *parent)
 
 ModelManagerInterface::~ModelManagerInterface()
 {
-    joinAllThreads(true);
+    Q_ASSERT(g_instance == this);
     m_cppQmlTypesUpdater.cancel();
     m_cppQmlTypesUpdater.waitForFinished();
 
-    QMutexLocker locker(&g_instanceMutex);
-    Q_ASSERT(g_instance == this);
-    g_instance = nullptr;
+    while (true) {
+        joinAllThreads(true);
+        // Keep these 2 mutexes in the same order as inside instanceForFuture()
+        QMutexLocker instanceLocker(&g_instanceMutex);
+        QMutexLocker futureLocker(&m_futuresMutex);
+        if (m_futureSynchronizer.isEmpty()) {
+            g_instance = nullptr;
+            return;
+        }
+    }
 }
 
 static QHash<QString, Dialect> defaultLanguageMapping()
@@ -655,6 +662,7 @@ QList<ModelManagerInterface::ProjectInfo> ModelManagerInterface::allProjectInfos
 
 bool ModelManagerInterface::isIdle() const
 {
+    QMutexLocker futureLocker(&m_futuresMutex);
     return m_futureSynchronizer.isEmpty();
 }