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: 96c860159b

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

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::joinAllThreads()
+{
+    while (true) {
+        FutureSynchronizer futureSynchronizer;
+        {
+            QMutexLocker locker(&m_futuresMutex);
+            futureSynchronizer = m_futureSynchronizer;
+            m_futureSynchronizer.clearFutures();
+        }
+        if (futureSynchronizer.isEmpty())
+            return;
+    }
+}
+
 void ModelManagerInterface::test_joinAllThreads()
 {
-    // Loop since futures can spawn more futures as they finish.
     while (true) {
-        QFuture<void> f;
+        joinAllThreads();
-        // get one future
+        // In order to process all onFinished handlers of finished futures
-        {
+        QCoreApplication::processEvents();
         QMutexLocker lock(&m_futuresMutex);
-            for (QFuture<void> &future : m_futures) {
+        // If handlers created new futures, repeat the loop
-                if (!future.isFinished() && !future.isCanceled()) {
+        if (m_futureSynchronizer.isEmpty())
-                    f = future;
+            return;
-                    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;
-        }
-    }
-    m_futures.clear();
 }
 
 void ModelManagerInterface::addFuture(const QFuture<void> &future)
 {
-    {
     QMutexLocker lock(&m_futuresMutex);
-        m_futures.append(future);
+    m_futureSynchronizer.addFuture(future);
-    }
-    cleanupFutures();
 }
 
 Document::Ptr ModelManagerInterface::ensuredGetDocumentForPath(const QString &filePath)

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;
 };

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,
+        Utils::onFinished(future, this, [this, libraryInfo, privatePlugin, libraryPath]
-                [this, libraryInfo, privatePlugin, libraryPath] (const QFuture<CppQmlTypesInfo>& future) {
+                          (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,

tests/auto/qml/codemodel/check/tst_check.cpp

@@ -101,9 +101,7 @@ void tst_Check::initTestCase()
         lPaths.maybeInsert(Utils::FilePath::fromString(p), Dialect::Qml);
     ModelManagerInterface::importScan(result, ModelManagerInterface::workingCopy(), lPaths,
                                       modelManager, false);
-    QCoreApplication::processEvents();
     modelManager->test_joinAllThreads();
-    QCoreApplication::processEvents();
 }
 
 static bool offsetComparator(const Message &lhs, const Message &rhs)

tests/auto/qml/codemodel/dependencies/tst_dependencies.cpp

@@ -151,9 +151,7 @@ void tst_Dependencies::test()
         lPaths.maybeInsert(Utils::FilePath::fromString(p), Dialect::Qml);
     ModelManagerInterface::importScan(result, ModelManagerInterface::workingCopy(), lPaths,
                                       ModelManagerInterface::instance(), false);
-    QCoreApplication::processEvents();
     ModelManagerInterface::instance()->test_joinAllThreads();
-    QCoreApplication::processEvents();
     TestData data = testData(filename);
     Document::MutablePtr doc = data.doc;
     int nExpectedSemanticMessages = data.semanticMessages;

tests/auto/qml/codemodel/importscheck/tst_importscheck.cpp

@@ -76,7 +76,6 @@ void scanDirectory(const QString &dir)
     paths.maybeInsert(Utils::FilePath::fromString(dir), Dialect::Qml);
     ModelManagerInterface::importScan(result, ModelManagerInterface::workingCopy(), paths,
                                       ModelManagerInterface::instance(), false);
-    QCoreApplication::processEvents();
     ModelManagerInterface::instance()->test_joinAllThreads();
     ViewerContext vCtx;
     vCtx.paths.append(dir);
@@ -273,7 +272,6 @@ void tst_ImportCheck::importTypes()
     modelManager->activateScan();
 
     modelManager->updateSourceFiles(QStringList(qmlFile), false);
-    QCoreApplication::processEvents();
     modelManager->test_joinAllThreads();
 
     Snapshot snapshot = modelManager->newestSnapshot();
@@ -288,7 +286,6 @@ void tst_ImportCheck::importTypes()
         return link();
     };
     getContext();
-    QCoreApplication::processEvents();
     modelManager->test_joinAllThreads();
     snapshot = modelManager->newestSnapshot();
     doc = snapshot.document(qmlFile);
@@ -367,7 +364,6 @@ void tst_ImportCheck::moduleMapping()
     scanDirectory(qtQuickImportPath);
 
     modelManager->updateSourceFiles(QStringList(qmlFile), false);
-    QCoreApplication::processEvents();
     modelManager->test_joinAllThreads();
 
     Snapshot snapshot = modelManager->newestSnapshot();
@@ -383,7 +379,6 @@ void tst_ImportCheck::moduleMapping()
         return link();
     };
     getContext();
-    QCoreApplication::processEvents();
     modelManager->test_joinAllThreads();
     snapshot = modelManager->newestSnapshot();
     doc = snapshot.document(qmlFile);