Refactor tst_joinAllThreads, to be used in ModelManager d'tor

The idea in this approach is that we only collect those futures,
which have resulted from runAsync. The assumption is that
all tasks associated with those futures may sooner or
later finish, without the need to call qApp->processEvents().

OTOH, we don't collect fake futures coming from Utils::onFinished,
as these requires the spinning event loop in order to deliver
the onFinished signal.

So, the new joinAllThreads() method waits for all collected
futures to finish. We also _do_ want canceled and not finished
futures to finish, since even when they are canceled,
they may still be running and using the internals
of possibly destructed ModelManager. This means, we are only
waiting for other threads to be finished, without reporting
their results to e.g. onFinished() handlers.

Some tests require that all onFinished handlers are also processed.
In order to achieve this, we create a loop inside
tst_joinAllThreads() method and we call joinAllThreads(), so
it will wait for all pending queue to finish, and then we call process
events, in order to let finished futures propagate their results
to their respective onFinished() handlers.
Some handlers may have stared another threads when being processed,
so we may expect that some new futures will appear.
So, after processing the events we check if any new events
appeared, and in this case we repeat the loop.
Otherwise, we finish synchronization.

Amends: 96c860159b862460e21be16a6e2839c0b591e016

Task-number: QTCREATORBUG-25350
Change-Id: I5e44150c55f6be00445a5695938482d948990c94
Reviewed-by: Fawzi Mohamed <fawzi.mohamed@qt.io>

3 files changed, 34 insertions, 51 deletions

diff --git a/src/libs/qmljs/qmljsmodelmanagerinterface.cpp b/src/libs/qmljs/qmljsmodelmanagerinterface.cpp
index 86e1e7046e9..e89b8ae23c8 100644
--- a/src/libs/qmljs/qmljsmodelmanagerinterface.cpp
+++ b/src/libs/qmljs/qmljsmodelmanagerinterface.cpp
@@ -106,6 +106,7 @@ ModelManagerInterface::ModelManagerInterface(QObject *parent)
       m_defaultImportPaths(environmentImportPaths()),
       m_pluginDumper(new PluginDumper(this))
 {
+    m_futureSynchronizer.setCancelOnWait(false);
     m_indexerDisabled = qEnvironmentVariableIsSet("QTC_NO_CODE_INDEXER");
 
     m_updateCppQmlTypesTimer = new QTimer(this);
@@ -307,20 +308,6 @@ void ModelManagerInterface::updateSourceFiles(const QStringList &files,
     refreshSourceFiles(files, emitDocumentOnDiskChanged);
 }
 
-void ModelManagerInterface::cleanupFutures()
-{
-    QMutexLocker lock(&m_futuresMutex);
-    const int maxFutures = 10;
-    if (m_futures.size() > maxFutures) {
-        const QList<QFuture<void>> futures = m_futures;
-        m_futures.clear();
-        for (const QFuture<void> &future : futures) {
-            if (!(future.isFinished() || future.isCanceled()))
-                m_futures.append(future);
-        }
-    }
-}
-
 QFuture<void> ModelManagerInterface::refreshSourceFiles(const QStringList &sourceFiles,
                                                         bool emitDocumentOnDiskChanged)
 {
@@ -355,9 +342,9 @@ QFuture<void> ModelManagerInterface::refreshSourceFiles(const QStringList &sourc
 
 void ModelManagerInterface::fileChangedOnDisk(const QString &path)
 {
-    Utils::runAsync(&ModelManagerInterface::parse,
+    addFuture(Utils::runAsync(&ModelManagerInterface::parse,
                     workingCopyInternal(), QStringList(path),
-                    this, Dialect(Dialect::AnyLanguage), true);
+                    this, Dialect(Dialect::AnyLanguage), true));
 }
 
 void ModelManagerInterface::removeFiles(const QStringList &files)
@@ -654,7 +641,7 @@ QList<ModelManagerInterface::ProjectInfo> ModelManagerInterface::allProjectInfos
 
 bool ModelManagerInterface::isIdle() const
 {
-    return m_futures.isEmpty();
+    return m_futureSynchronizer.isEmpty();
 }
 
 void ModelManagerInterface::emitDocumentChangedOnDisk(Document::Ptr doc)
@@ -1571,41 +1558,37 @@ void ModelManagerInterface::setDefaultVContext(const ViewerContext &vContext)
     m_defaultVContexts[vContext.language] = vContext;
 }
 
-void ModelManagerInterface::test_joinAllThreads()
+void ModelManagerInterface::joinAllThreads()
 {
-    // Loop since futures can spawn more futures as they finish.
     while (true) {
-        QFuture<void> f;
-        // get one future
+        FutureSynchronizer futureSynchronizer;
         {
-            QMutexLocker lock(&m_futuresMutex);
-            for (QFuture<void> &future : m_futures) {
-                if (!future.isFinished() && !future.isCanceled()) {
-                    f = future;
-                    break;
-                }
-            }
-        }
-        if (!f.isFinished() && !f.isCanceled()) {
-            f.waitForFinished();
-
-            // Some futures trigger more futures from connected signals
-            // and in tests, we care about finishing all of these too.
-            QEventLoop().processEvents();
-        } else {
-            break;
+            QMutexLocker locker(&m_futuresMutex);
+            futureSynchronizer = m_futureSynchronizer;
+            m_futureSynchronizer.clearFutures();
         }
+        if (futureSynchronizer.isEmpty())
+            return;
     }
-    m_futures.clear();
 }
 
-void ModelManagerInterface::addFuture(const QFuture<void> &future)
+void ModelManagerInterface::test_joinAllThreads()
 {
-    {
+    while (true) {
+        joinAllThreads();
+        // In order to process all onFinished handlers of finished futures
+        QCoreApplication::processEvents();
         QMutexLocker lock(&m_futuresMutex);
-        m_futures.append(future);
+        // If handlers created new futures, repeat the loop
+        if (m_futureSynchronizer.isEmpty())
+            return;
     }
-    cleanupFutures();
+}
+
+void ModelManagerInterface::addFuture(const QFuture<void> &future)
+{
+    QMutexLocker lock(&m_futuresMutex);
+    m_futureSynchronizer.addFuture(future);
 }
 
 Document::Ptr ModelManagerInterface::ensuredGetDocumentForPath(const QString &filePath)
diff --git a/src/libs/qmljs/qmljsmodelmanagerinterface.h b/src/libs/qmljs/qmljsmodelmanagerinterface.h
index f6142255469..e31c69b84e4 100644
--- a/src/libs/qmljs/qmljsmodelmanagerinterface.h
+++ b/src/libs/qmljs/qmljsmodelmanagerinterface.h
@@ -32,6 +32,7 @@
 
 #include <cplusplus/CppDocument.h>
 #include <utils/environment.h>
+#include <utils/futuresynchronizer.h>
 #include <utils/qrcparser.h>
 
 #include <QFuture>
@@ -243,7 +244,7 @@ protected:
     void setDefaultProject(const ProjectInfo &pInfo, ProjectExplorer::Project *p);
 
 private:
-    void cleanupFutures();
+    void joinAllThreads();
     void iterateQrcFiles(ProjectExplorer::Project *project,
                          QrcResourceSelector resources,
                          const std::function<void(Utils::QrcParser::ConstPtr)> &callback);
@@ -280,7 +281,7 @@ private:
     PluginDumper *m_pluginDumper = nullptr;
 
     mutable QMutex m_futuresMutex;
-    QList<QFuture<void>> m_futures;
+    Utils::FutureSynchronizer m_futureSynchronizer;
 
     bool m_indexerDisabled = false;
 };
diff --git a/src/libs/qmljs/qmljsplugindumper.cpp b/src/libs/qmljs/qmljsplugindumper.cpp
index a98f7550d62..376166559f3 100644
--- a/src/libs/qmljs/qmljsplugindumper.cpp
+++ b/src/libs/qmljs/qmljsplugindumper.cpp
@@ -300,9 +300,10 @@ void PluginDumper::qmlPluginTypeDumpDone(int exitCode)
                                                         QLatin1String("<dump of ") + libraryPath + QLatin1Char('>'));
             future.reportFinished(&infos);
         });
+        m_modelManager->addFuture(future);
 
-        auto finalFuture = Utils::onFinished(future, this,
-                [this, libraryInfo, privatePlugin, libraryPath] (const QFuture<CppQmlTypesInfo>& future) {
+        Utils::onFinished(future, this, [this, libraryInfo, privatePlugin, libraryPath]
+                          (const QFuture<CppQmlTypesInfo>& future) {
             CppQmlTypesInfo infos = future.result();
 
             LibraryInfo libInfo = libraryInfo;
@@ -325,7 +326,6 @@ void PluginDumper::qmlPluginTypeDumpDone(int exitCode)
 
             m_modelManager->updateLibraryInfo(libraryPath, libInfo);
         });
-        m_modelManager->addFuture(finalFuture);
     } else {
         libraryInfo.setPluginTypeInfoStatus(LibraryInfo::DumpDone);
         libraryInfo.updateFingerprint();
@@ -395,6 +395,7 @@ QFuture<PluginDumper::QmlTypeDescription> PluginDumper::loadQmlTypeDescription(c
 
         future.reportFinished(&result);
     });
+    m_modelManager->addFuture(future);
 
     return future;
 }
@@ -598,12 +599,12 @@ void PluginDumper::loadQmltypesFile(const QStringList &qmltypesFilePaths,
                                     const QString &libraryPath,
                                     QmlJS::LibraryInfo libraryInfo)
 {
-    auto future = Utils::onFinished(loadQmlTypeDescription(qmltypesFilePaths), this, [=](const QFuture<PluginDumper::QmlTypeDescription> &typesFuture)
+    Utils::onFinished(loadQmlTypeDescription(qmltypesFilePaths), this, [=](const QFuture<PluginDumper::QmlTypeDescription> &typesFuture)
     {
         PluginDumper::QmlTypeDescription typesResult = typesFuture.result();
         if (!typesResult.dependencies.isEmpty())
         {
-            auto depFuture = Utils::onFinished(loadDependencies(typesResult.dependencies, QSharedPointer<QSet<QString>>()), this,
+            Utils::onFinished(loadDependencies(typesResult.dependencies, QSharedPointer<QSet<QString>>()), this,
                               [typesResult, libraryInfo, libraryPath, this] (const QFuture<PluginDumper::DependencyInfo> &loadFuture)
             {
                 PluginDumper::DependencyInfo loadResult = loadFuture.result();
@@ -621,7 +622,6 @@ void PluginDumper::loadQmltypesFile(const QStringList &qmltypesFilePaths,
                                    typesResult.moduleApis, objects);
                 m_modelManager->updateLibraryInfo(libraryPath, libInfo);
             });
-            m_modelManager->addFuture(depFuture);
         } else {
             QmlJS::LibraryInfo libInfo = libraryInfo;
             prepareLibraryInfo(libInfo, libraryPath, typesResult.dependencies,
@@ -630,7 +630,6 @@ void PluginDumper::loadQmltypesFile(const QStringList &qmltypesFilePaths,
             m_modelManager->updateLibraryInfo(libraryPath, libInfo);
         }
     });
-    m_modelManager->addFuture(future);
 }
 
 void PluginDumper::runQmlDump(const QmlJS::ModelManagerInterface::ProjectInfo &info,