Fix CppProjectUpdater cancelAndWaitForFinished

The code was pushing an additional QFutureInterface through the whole
chain of functions, which was used for canceling. But since it was never
started (and never finished, and never used for reporting results),
calling waitForFinshed on it never had any effect with Qt5 and locks up
with Qt6.

Instead of using a separate QFutureInterface, use the actual QFuture
that is available and intended for it.

Fixes: QTCREATORBUG-24902
Change-Id: I5a49bcecc9cf70fbffa93aee4293004f9369df58
Reviewed-by: Jarek Kobus <jaroslaw.kobus@qt.io>
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/plugins/cpptools/builtinindexingsupport.cpp

@@ -135,7 +135,6 @@ void classifyFiles(const QSet<QString> &files, QStringList *headers, QStringList
 }
 
 void indexFindErrors(QFutureInterface<void> &indexingFuture,
-                     const QFutureInterface<void> &superFuture,
                      const ParseParams params)
 {
     QStringList sources, headers;
@@ -149,7 +148,7 @@ void indexFindErrors(QFutureInterface<void> &indexingFuture,
     timer.start();
 
     for (int i = 0, end = files.size(); i < end ; ++i) {
-        if (indexingFuture.isCanceled() || superFuture.isCanceled())
+        if (indexingFuture.isCanceled())
             break;
 
         const QString file = files.at(i);
@@ -179,7 +178,6 @@ void indexFindErrors(QFutureInterface<void> &indexingFuture,
 }
 
 void index(QFutureInterface<void> &indexingFuture,
-           const QFutureInterface<void> &superFuture,
            const ParseParams params)
 {
     QScopedPointer<CppSourceProcessor> sourceProcessor(CppModelManager::createSourceProcessor());
@@ -209,7 +207,7 @@ void index(QFutureInterface<void> &indexingFuture,
 
     qCDebug(indexerLog) << "About to index" << files.size() << "files.";
     for (int i = 0; i < files.size(); ++i) {
-        if (indexingFuture.isCanceled() || superFuture.isCanceled())
+        if (indexingFuture.isCanceled())
             break;
 
         const QString fileName = files.at(i);
@@ -243,9 +241,7 @@ void index(QFutureInterface<void> &indexingFuture,
     qCDebug(indexerLog) << "Indexing finished.";
 }
 
-void parse(QFutureInterface<void> &indexingFuture,
+void parse(QFutureInterface<void> &indexingFuture, const ParseParams params)
-           const QFutureInterface<void> &superFuture,
-           const ParseParams params)
 {
     const QSet<QString> &files = params.sourceFiles;
     if (files.isEmpty())
@@ -254,9 +250,9 @@ void parse(QFutureInterface<void> &indexingFuture,
     indexingFuture.setProgressRange(0, files.size());
 
     if (FindErrorsIndexing)
-        indexFindErrors(indexingFuture, superFuture, params);
+        indexFindErrors(indexingFuture, params);
     else
-        index(indexingFuture, superFuture, params);
+        index(indexingFuture, params);
 
     indexingFuture.setProgressValue(files.size());
     CppModelManager::instance()->finishedRefreshingSourceFiles(files);
@@ -349,9 +345,7 @@ BuiltinIndexingSupport::BuiltinIndexingSupport()
 BuiltinIndexingSupport::~BuiltinIndexingSupport() = default;
 
 QFuture<void> BuiltinIndexingSupport::refreshSourceFiles(
-        const QFutureInterface<void> &superFuture,
+    const QSet<QString> &sourceFiles, CppModelManager::ProgressNotificationMode mode)
-        const QSet<QString> &sourceFiles,
-        CppModelManager::ProgressNotificationMode mode)
 {
     CppModelManager *mgr = CppModelManager::instance();
 
@@ -361,7 +355,7 @@ QFuture<void> BuiltinIndexingSupport::refreshSourceFiles(
     params.workingCopy = mgr->workingCopy();
     params.sourceFiles = sourceFiles;
 
-    QFuture<void> result = Utils::runAsync(mgr->sharedThreadPool(), parse, superFuture, params);
+    QFuture<void> result = Utils::runAsync(mgr->sharedThreadPool(), parse, params);
 
     if (m_synchronizer.futures().size() > 10) {
         QList<QFuture<void> > futures = m_synchronizer.futures();

src/plugins/cpptools/builtinindexingsupport.h

@@ -38,8 +38,7 @@ public:
     BuiltinIndexingSupport();
     ~BuiltinIndexingSupport() override;
 
-    QFuture<void> refreshSourceFiles(const QFutureInterface<void> &superFuture,
+    QFuture<void> refreshSourceFiles(const QSet<QString> &sourceFiles,
-                                     const QSet<QString> &sourceFiles,
                                      CppModelManager::ProgressNotificationMode mode) override;
     SymbolSearcher *createSymbolSearcher(const SymbolSearcher::Parameters &parameters,
                                          const QSet<QString> &fileNames) override;

src/plugins/cpptools/cppindexingsupport.h

@@ -78,9 +78,9 @@ class CPPTOOLS_EXPORT CppIndexingSupport
 public:
     virtual ~CppIndexingSupport() = 0;
 
-    virtual QFuture<void> refreshSourceFiles(const QFutureInterface<void> &superFuture,
+    virtual QFuture<void> refreshSourceFiles(const QSet<QString> &sourceFiles,
-                                             const QSet<QString> &sourceFiles,
+                                             CppModelManager::ProgressNotificationMode mode)
-                                             CppModelManager::ProgressNotificationMode mode) = 0;
+        = 0;
     virtual SymbolSearcher *createSymbolSearcher(const SymbolSearcher::Parameters &parameters,
                                                  const QSet<QString> &fileNames) = 0;
 };

src/plugins/cpptools/cppmodelmanager.cpp

@@ -929,14 +929,6 @@ static QSet<QString> tooBigFilesRemoved(const QSet<QString> &files, int fileSize
 
 QFuture<void> CppModelManager::updateSourceFiles(const QSet<QString> &sourceFiles,
                                                  ProgressNotificationMode mode)
-{
-    const QFutureInterface<void> dummy;
-    return updateSourceFiles(dummy, sourceFiles, mode);
-}
-
-QFuture<void> CppModelManager::updateSourceFiles(const QFutureInterface<void> &superFuture,
-                                                 const QSet<QString> &sourceFiles,
-                                                 ProgressNotificationMode mode)
 {
     if (sourceFiles.isEmpty() || !d->m_indexerEnabled)
         return QFuture<void>();
@@ -944,8 +936,8 @@ QFuture<void> CppModelManager::updateSourceFiles(const QFutureInterface<void> &s
     const QSet<QString> filteredFiles = tooBigFilesRemoved(sourceFiles, indexerFileSizeLimitInMb());
 
     if (d->m_indexingSupporter)
-        d->m_indexingSupporter->refreshSourceFiles(superFuture, filteredFiles, mode);
+        d->m_indexingSupporter->refreshSourceFiles(filteredFiles, mode);
-    return d->m_internalIndexingSupport->refreshSourceFiles(superFuture, filteredFiles, mode);
+    return d->m_internalIndexingSupport->refreshSourceFiles(filteredFiles, mode);
 }
 
 QList<ProjectInfo> CppModelManager::projectInfos() const
@@ -1078,25 +1070,23 @@ void CppModelManager::recalculateProjectPartMappings()
     d->m_symbolFinder.clearCache();
 }
 
-void CppModelManager::watchForCanceledProjectIndexer(const QVector<QFuture<void>> &futures,
+void CppModelManager::watchForCanceledProjectIndexer(const QFuture<void> &future,
                                                      ProjectExplorer::Project *project)
 {
-    for (const QFuture<void> &future : futures) {
+    if (future.isCanceled() || future.isFinished())
-        if (future.isCanceled() || future.isFinished())
+        return;
-            continue;
 
-        auto watcher = new QFutureWatcher<void>(this);
+    auto watcher = new QFutureWatcher<void>(this);
-        connect(watcher, &QFutureWatcher<void>::canceled, this, [this, project, watcher]() {
+    connect(watcher, &QFutureWatcher<void>::canceled, this, [this, project, watcher]() {
-            if (d->m_projectToIndexerCanceled.contains(project)) // Project not yet removed
+        if (d->m_projectToIndexerCanceled.contains(project)) // Project not yet removed
-                d->m_projectToIndexerCanceled.insert(project, true);
+            d->m_projectToIndexerCanceled.insert(project, true);
-            watcher->deleteLater();
+        watcher->deleteLater();
-        });
+    });
-        connect(watcher, &QFutureWatcher<void>::finished, this, [this, project, watcher]() {
+    connect(watcher, &QFutureWatcher<void>::finished, this, [this, project, watcher]() {
-            d->m_projectToIndexerCanceled.remove(project);
+        d->m_projectToIndexerCanceled.remove(project);
-            watcher->deleteLater();
+        watcher->deleteLater();
-        });
+    });
-        watcher->setFuture(future);
+    watcher->setFuture(future);
-    }
 }
 
 void CppModelManager::updateCppEditorDocuments(bool projectsUpdated) const
@@ -1128,8 +1118,7 @@ void CppModelManager::updateCppEditorDocuments(bool projectsUpdated) const
     }
 }
 
-QFuture<void> CppModelManager::updateProjectInfo(QFutureInterface<void> &futureInterface,
+QFuture<void> CppModelManager::updateProjectInfo(const ProjectInfo &newProjectInfo)
-                                                 const ProjectInfo &newProjectInfo)
 {
     if (!newProjectInfo.isValid())
         return QFuture<void>();
@@ -1221,12 +1210,12 @@ QFuture<void> CppModelManager::updateProjectInfo(QFutureInterface<void> &futureI
     updateCppEditorDocuments(/*projectsUpdated = */ true);
 
     // Trigger reindexing
-    const QFuture<void> indexingFuture = updateSourceFiles(futureInterface, filesToReindex,
+    const QFuture<void> indexingFuture = updateSourceFiles(filesToReindex,
                                                            ForcedProgressNotification);
     if (!filesToReindex.isEmpty()) {
         d->m_projectToIndexerCanceled.insert(project, false);
     }
-    watchForCanceledProjectIndexer({futureInterface.future(), indexingFuture}, project);
+    watchForCanceledProjectIndexer(indexingFuture, project);
     return indexingFuture;
 }
 

src/plugins/cpptools/cppmodelmanager.h

@@ -106,17 +106,13 @@ public:
 
     QFuture<void> updateSourceFiles(const QSet<QString> &sourceFiles,
                                     ProgressNotificationMode mode = ReservedProgressNotification);
-    QFuture<void> updateSourceFiles(const QFutureInterface<void> &superFuture,
-                                    const QSet<QString> &sourceFiles,
-                                    ProgressNotificationMode mode = ReservedProgressNotification);
     void updateCppEditorDocuments(bool projectsUpdated = false) const;
     WorkingCopy workingCopy() const;
     QByteArray codeModelConfiguration() const;
 
     QList<ProjectInfo> projectInfos() const;
     ProjectInfo projectInfo(ProjectExplorer::Project *project) const;
-    QFuture<void> updateProjectInfo(QFutureInterface<void> &futureInterface,
+    QFuture<void> updateProjectInfo(const ProjectInfo &newProjectInfo);
-                                    const ProjectInfo &newProjectInfo);
 
     /// \return The project part with the given project file
     ProjectPart::Ptr projectPartForId(const QString &projectPartId) const override;
@@ -274,7 +270,7 @@ private:
     void initializeBuiltinModelManagerSupport();
     void delayedGC();
     void recalculateProjectPartMappings();
-    void watchForCanceledProjectIndexer(const QVector<QFuture<void> > &futures,
+    void watchForCanceledProjectIndexer(const QFuture<void> &future,
                                         ProjectExplorer::Project *project);
 
     void replaceSnapshot(const CPlusPlus::Snapshot &newSnapshot);

src/plugins/cpptools/cppmodelmanager_test.cpp

@@ -190,8 +190,7 @@ void CppToolsPlugin::test_modelmanager_paths_are_clean()
                          {testDataDir.frameworksDir(false), HeaderPathType::Framework}};
     pi.appendProjectPart(part);
 
-    QFutureInterface<void> dummy;
+    mm->updateProjectInfo(pi);
-    mm->updateProjectInfo(dummy, pi);
 
     ProjectExplorer::HeaderPaths headerPaths = mm->headerPaths();
     QCOMPARE(headerPaths.size(), 2);
@@ -223,8 +222,7 @@ void CppToolsPlugin::test_modelmanager_framework_headers()
     part->files << ProjectFile(source, ProjectFile::CXXSource);
     pi.appendProjectPart(part);
 
-    QFutureInterface<void> dummy;
+    mm->updateProjectInfo(pi).waitForFinished();
-    mm->updateProjectInfo(dummy, pi).waitForFinished();
     QCoreApplication::processEvents();
 
     QVERIFY(mm->snapshot().contains(source));
@@ -323,8 +321,7 @@ void CppToolsPlugin::test_modelmanager_refresh_several_times()
     part->files.append(ProjectFile(testHeader2, ProjectFile::CXXHeader));
     part->files.append(ProjectFile(testCpp, ProjectFile::CXXSource));
     pi.appendProjectPart(part);
-    QFutureInterface<void> dummy;
+    mm->updateProjectInfo(pi);
-    mm->updateProjectInfo(dummy, pi);
 
     CPlusPlus::Snapshot snapshot;
     QSet<QString> refreshedFiles;
@@ -387,8 +384,7 @@ void CppToolsPlugin::test_modelmanager_refresh_test_for_changes()
 
     // Reindexing triggers a reparsing thread
     helper.resetRefreshedSourceFiles();
-    QFutureInterface<void> dummy;
+    QFuture<void> firstFuture = mm->updateProjectInfo(pi);
-    QFuture<void> firstFuture = mm->updateProjectInfo(dummy, pi);
     QVERIFY(firstFuture.isStarted() || firstFuture.isRunning());
     firstFuture.waitForFinished();
     const QSet<QString> refreshedFiles = helper.waitForRefreshedSourceFiles();
@@ -396,7 +392,7 @@ void CppToolsPlugin::test_modelmanager_refresh_test_for_changes()
     QVERIFY(refreshedFiles.contains(testCpp));
 
     // No reindexing since nothing has changed
-    QFuture<void> subsequentFuture = mm->updateProjectInfo(dummy, pi);
+    QFuture<void> subsequentFuture = mm->updateProjectInfo(pi);
     QVERIFY(subsequentFuture.isCanceled() && subsequentFuture.isFinished());
 }
 

src/plugins/cpptools/cppprojectinfogenerator.cpp

@@ -42,9 +42,8 @@ using namespace ProjectExplorer;
 namespace CppTools {
 namespace Internal {
 
-ProjectInfoGenerator::ProjectInfoGenerator(
+ProjectInfoGenerator::ProjectInfoGenerator(const QFutureInterface<ProjectInfo> &futureInterface,
-    const QFutureInterface<void> &futureInterface,
+                                           const ProjectUpdateInfo &projectUpdateInfo)
-    const ProjectUpdateInfo &projectUpdateInfo)
     : m_futureInterface(futureInterface)
     , m_projectUpdateInfo(projectUpdateInfo)
 {

src/plugins/cpptools/cppprojectinfogenerator.h

@@ -36,7 +36,7 @@ namespace Internal {
 class ProjectInfoGenerator
 {
 public:
-    ProjectInfoGenerator(const QFutureInterface<void> &futureInterface,
+    ProjectInfoGenerator(const QFutureInterface<ProjectInfo> &futureInterface,
                          const ProjectExplorer::ProjectUpdateInfo &projectUpdateInfo);
 
     ProjectInfo generate();
@@ -52,7 +52,7 @@ private:
                                        Utils::LanguageExtensions languageExtensions);
 
 private:
-    const QFutureInterface<void> m_futureInterface;
+    const QFutureInterface<ProjectInfo> m_futureInterface;
     const ProjectExplorer::ProjectUpdateInfo &m_projectUpdateInfo;
     bool m_cToolchainMissing = false;
     bool m_cxxToolchainMissing = false;

src/plugins/cpptools/cppprojectupdater.cpp

@@ -37,8 +37,10 @@ namespace CppTools {
 
 CppProjectUpdater::CppProjectUpdater()
 {
-    connect(&m_generateFutureWatcher, &QFutureWatcher<void>::finished,
+    connect(&m_generateFutureWatcher,
-            this, &CppProjectUpdater::onProjectInfoGenerated);
+            &QFutureWatcher<ProjectInfo>::finished,
+            this,
+            &CppProjectUpdater::onProjectInfoGenerated);
 }
 
 CppProjectUpdater::~CppProjectUpdater()
@@ -50,7 +52,6 @@ void CppProjectUpdater::update(const ProjectExplorer::ProjectUpdateInfo &project
 {
     // Stop previous update.
     cancelAndWaitForFinished();
-    m_futureInterface = QFutureInterface<void>();
 
     m_projectUpdateInfo = projectUpdateInfo;
 
@@ -60,26 +61,30 @@ void CppProjectUpdater::update(const ProjectExplorer::ProjectUpdateInfo &project
             this, &CppProjectUpdater::onToolChainRemoved);
 
     // Run the project info generator in a worker thread and continue if that one is finished.
-    const QFuture<ProjectInfo> future = Utils::runAsync([=]() {
+    m_generateFuture = Utils::runAsync([=](QFutureInterface<ProjectInfo> &futureInterface) {
         ProjectUpdateInfo fullProjectUpdateInfo = projectUpdateInfo;
         if (fullProjectUpdateInfo.rppGenerator)
             fullProjectUpdateInfo.rawProjectParts = fullProjectUpdateInfo.rppGenerator();
-        Internal::ProjectInfoGenerator generator(m_futureInterface, fullProjectUpdateInfo);
+        Internal::ProjectInfoGenerator generator(futureInterface, fullProjectUpdateInfo);
-        return generator.generate();
+        futureInterface.reportResult(generator.generate());
     });
-    m_generateFutureWatcher.setFuture(future);
+    m_generateFutureWatcher.setFuture(m_generateFuture);
 }
 
 void CppProjectUpdater::cancel()
 {
-    disconnect(&m_generateFutureWatcher);
+    m_generateFutureWatcher.setFuture({});
-    m_futureInterface.cancel();
+    m_generateFuture.cancel();
+    m_updateFuture.cancel();
 }
 
 void CppProjectUpdater::cancelAndWaitForFinished()
 {
     cancel();
-    m_futureInterface.waitForFinished();
+    if (m_generateFuture.isRunning())
+        m_generateFuture.waitForFinished();
+    if (m_updateFuture.isRunning())
+        m_updateFuture.waitForFinished();
 }
 
 void CppProjectUpdater::onToolChainRemoved(ProjectExplorer::ToolChain *t)
@@ -96,11 +101,11 @@ void CppProjectUpdater::onProjectInfoGenerated()
     disconnect(ToolChainManager::instance(), &ToolChainManager::toolChainRemoved,
                this, &CppProjectUpdater::onToolChainRemoved);
 
-    if (m_futureInterface.isCanceled())
+    if (m_generateFutureWatcher.isCanceled() || m_generateFutureWatcher.future().resultCount() < 1)
         return;
 
-    QFuture<void> future = CppModelManager::instance()
+    m_updateFuture = CppModelManager::instance()->updateProjectInfo(
-            ->updateProjectInfo(m_futureInterface, m_generateFutureWatcher.result());
+        m_generateFutureWatcher.result());
 }
 
 CppProjectUpdaterFactory::CppProjectUpdaterFactory()

src/plugins/cpptools/cppprojectupdater.h

@@ -67,7 +67,8 @@ private:
 private:
     ProjectExplorer::ProjectUpdateInfo m_projectUpdateInfo;
 
-    QFutureInterface<void> m_futureInterface;
+    QFuture<ProjectInfo> m_generateFuture;
+    QFuture<void> m_updateFuture;
     QFutureWatcher<ProjectInfo> m_generateFutureWatcher;
 };
 

src/plugins/cpptools/modelmanagertesthelper.cpp

@@ -93,8 +93,7 @@ ModelManagerTestHelper::Project *ModelManagerTestHelper::createProject(const QSt
 QSet<QString> ModelManagerTestHelper::updateProjectInfo(const CppTools::ProjectInfo &projectInfo)
 {
     resetRefreshedSourceFiles();
-    QFutureInterface<void> dummy;
+    CppModelManager::instance()->updateProjectInfo(projectInfo).waitForFinished();
-    CppModelManager::instance()->updateProjectInfo(dummy, projectInfo).waitForFinished();
     QCoreApplication::processEvents();
     return waitForRefreshedSourceFiles();
 }

tests/unit/unittest/cppprojectinfogenerator-test.cpp

@@ -163,7 +163,7 @@ void ProjectInfoGenerator::SetUp()
 
 ProjectInfo ProjectInfoGenerator::generate()
 {
-    QFutureInterface<void> fi;
+    QFutureInterface<ProjectInfo> fi;
     ProjectExplorer::ConcreteToolChain aToolChain;
 
     projectUpdateInfo.rawProjectParts += rawProjectPart;