From 6d7e5eb8d17194b124c8986d670ff96fda96f0f3 Mon Sep 17 00:00:00 2001 From: Eike Ziller Date: Tue, 10 Nov 2020 17:55:54 +0100 Subject: [PATCH] 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 Reviewed-by: Christian Kandeler --- .../cpptools/builtinindexingsupport.cpp | 20 +++----- src/plugins/cpptools/builtinindexingsupport.h | 3 +- src/plugins/cpptools/cppindexingsupport.h | 6 +-- src/plugins/cpptools/cppmodelmanager.cpp | 49 +++++++------------ src/plugins/cpptools/cppmodelmanager.h | 8 +-- src/plugins/cpptools/cppmodelmanager_test.cpp | 14 ++---- .../cpptools/cppprojectinfogenerator.cpp | 5 +- .../cpptools/cppprojectinfogenerator.h | 4 +- src/plugins/cpptools/cppprojectupdater.cpp | 31 +++++++----- src/plugins/cpptools/cppprojectupdater.h | 3 +- .../cpptools/modelmanagertesthelper.cpp | 3 +- .../unittest/cppprojectinfogenerator-test.cpp | 2 +- 12 files changed, 63 insertions(+), 85 deletions(-) diff --git a/src/plugins/cpptools/builtinindexingsupport.cpp b/src/plugins/cpptools/builtinindexingsupport.cpp index cfb3be635fe..99d50876a61 100644 --- a/src/plugins/cpptools/builtinindexingsupport.cpp +++ b/src/plugins/cpptools/builtinindexingsupport.cpp @@ -135,7 +135,6 @@ void classifyFiles(const QSet &files, QStringList *headers, QStringList } void indexFindErrors(QFutureInterface &indexingFuture, - const QFutureInterface &superFuture, const ParseParams params) { QStringList sources, headers; @@ -149,7 +148,7 @@ void indexFindErrors(QFutureInterface &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 &indexingFuture, } void index(QFutureInterface &indexingFuture, - const QFutureInterface &superFuture, const ParseParams params) { QScopedPointer sourceProcessor(CppModelManager::createSourceProcessor()); @@ -209,7 +207,7 @@ void index(QFutureInterface &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 &indexingFuture, qCDebug(indexerLog) << "Indexing finished."; } -void parse(QFutureInterface &indexingFuture, - const QFutureInterface &superFuture, - const ParseParams params) +void parse(QFutureInterface &indexingFuture, const ParseParams params) { const QSet &files = params.sourceFiles; if (files.isEmpty()) @@ -254,9 +250,9 @@ void parse(QFutureInterface &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 BuiltinIndexingSupport::refreshSourceFiles( - const QFutureInterface &superFuture, - const QSet &sourceFiles, - CppModelManager::ProgressNotificationMode mode) + const QSet &sourceFiles, CppModelManager::ProgressNotificationMode mode) { CppModelManager *mgr = CppModelManager::instance(); @@ -361,7 +355,7 @@ QFuture BuiltinIndexingSupport::refreshSourceFiles( params.workingCopy = mgr->workingCopy(); params.sourceFiles = sourceFiles; - QFuture result = Utils::runAsync(mgr->sharedThreadPool(), parse, superFuture, params); + QFuture result = Utils::runAsync(mgr->sharedThreadPool(), parse, params); if (m_synchronizer.futures().size() > 10) { QList > futures = m_synchronizer.futures(); diff --git a/src/plugins/cpptools/builtinindexingsupport.h b/src/plugins/cpptools/builtinindexingsupport.h index 26e0a22dd3b..9a61d60ec96 100644 --- a/src/plugins/cpptools/builtinindexingsupport.h +++ b/src/plugins/cpptools/builtinindexingsupport.h @@ -38,8 +38,7 @@ public: BuiltinIndexingSupport(); ~BuiltinIndexingSupport() override; - QFuture refreshSourceFiles(const QFutureInterface &superFuture, - const QSet &sourceFiles, + QFuture refreshSourceFiles(const QSet &sourceFiles, CppModelManager::ProgressNotificationMode mode) override; SymbolSearcher *createSymbolSearcher(const SymbolSearcher::Parameters ¶meters, const QSet &fileNames) override; diff --git a/src/plugins/cpptools/cppindexingsupport.h b/src/plugins/cpptools/cppindexingsupport.h index e3d9eb257e2..1a77dc822c8 100644 --- a/src/plugins/cpptools/cppindexingsupport.h +++ b/src/plugins/cpptools/cppindexingsupport.h @@ -78,9 +78,9 @@ class CPPTOOLS_EXPORT CppIndexingSupport public: virtual ~CppIndexingSupport() = 0; - virtual QFuture refreshSourceFiles(const QFutureInterface &superFuture, - const QSet &sourceFiles, - CppModelManager::ProgressNotificationMode mode) = 0; + virtual QFuture refreshSourceFiles(const QSet &sourceFiles, + CppModelManager::ProgressNotificationMode mode) + = 0; virtual SymbolSearcher *createSymbolSearcher(const SymbolSearcher::Parameters ¶meters, const QSet &fileNames) = 0; }; diff --git a/src/plugins/cpptools/cppmodelmanager.cpp b/src/plugins/cpptools/cppmodelmanager.cpp index ca8fe286868..7315b303df8 100644 --- a/src/plugins/cpptools/cppmodelmanager.cpp +++ b/src/plugins/cpptools/cppmodelmanager.cpp @@ -929,14 +929,6 @@ static QSet tooBigFilesRemoved(const QSet &files, int fileSize QFuture CppModelManager::updateSourceFiles(const QSet &sourceFiles, ProgressNotificationMode mode) -{ - const QFutureInterface dummy; - return updateSourceFiles(dummy, sourceFiles, mode); -} - -QFuture CppModelManager::updateSourceFiles(const QFutureInterface &superFuture, - const QSet &sourceFiles, - ProgressNotificationMode mode) { if (sourceFiles.isEmpty() || !d->m_indexerEnabled) return QFuture(); @@ -944,8 +936,8 @@ QFuture CppModelManager::updateSourceFiles(const QFutureInterface &s const QSet filteredFiles = tooBigFilesRemoved(sourceFiles, indexerFileSizeLimitInMb()); if (d->m_indexingSupporter) - d->m_indexingSupporter->refreshSourceFiles(superFuture, filteredFiles, mode); - return d->m_internalIndexingSupport->refreshSourceFiles(superFuture, filteredFiles, mode); + d->m_indexingSupporter->refreshSourceFiles(filteredFiles, mode); + return d->m_internalIndexingSupport->refreshSourceFiles(filteredFiles, mode); } QList CppModelManager::projectInfos() const @@ -1078,25 +1070,23 @@ void CppModelManager::recalculateProjectPartMappings() d->m_symbolFinder.clearCache(); } -void CppModelManager::watchForCanceledProjectIndexer(const QVector> &futures, +void CppModelManager::watchForCanceledProjectIndexer(const QFuture &future, ProjectExplorer::Project *project) { - for (const QFuture &future : futures) { - if (future.isCanceled() || future.isFinished()) - continue; + if (future.isCanceled() || future.isFinished()) + return; - auto watcher = new QFutureWatcher(this); - connect(watcher, &QFutureWatcher::canceled, this, [this, project, watcher]() { - if (d->m_projectToIndexerCanceled.contains(project)) // Project not yet removed - d->m_projectToIndexerCanceled.insert(project, true); - watcher->deleteLater(); - }); - connect(watcher, &QFutureWatcher::finished, this, [this, project, watcher]() { - d->m_projectToIndexerCanceled.remove(project); - watcher->deleteLater(); - }); - watcher->setFuture(future); - } + auto watcher = new QFutureWatcher(this); + connect(watcher, &QFutureWatcher::canceled, this, [this, project, watcher]() { + if (d->m_projectToIndexerCanceled.contains(project)) // Project not yet removed + d->m_projectToIndexerCanceled.insert(project, true); + watcher->deleteLater(); + }); + connect(watcher, &QFutureWatcher::finished, this, [this, project, watcher]() { + d->m_projectToIndexerCanceled.remove(project); + watcher->deleteLater(); + }); + watcher->setFuture(future); } void CppModelManager::updateCppEditorDocuments(bool projectsUpdated) const @@ -1128,8 +1118,7 @@ void CppModelManager::updateCppEditorDocuments(bool projectsUpdated) const } } -QFuture CppModelManager::updateProjectInfo(QFutureInterface &futureInterface, - const ProjectInfo &newProjectInfo) +QFuture CppModelManager::updateProjectInfo(const ProjectInfo &newProjectInfo) { if (!newProjectInfo.isValid()) return QFuture(); @@ -1221,12 +1210,12 @@ QFuture CppModelManager::updateProjectInfo(QFutureInterface &futureI updateCppEditorDocuments(/*projectsUpdated = */ true); // Trigger reindexing - const QFuture indexingFuture = updateSourceFiles(futureInterface, filesToReindex, + const QFuture indexingFuture = updateSourceFiles(filesToReindex, ForcedProgressNotification); if (!filesToReindex.isEmpty()) { d->m_projectToIndexerCanceled.insert(project, false); } - watchForCanceledProjectIndexer({futureInterface.future(), indexingFuture}, project); + watchForCanceledProjectIndexer(indexingFuture, project); return indexingFuture; } diff --git a/src/plugins/cpptools/cppmodelmanager.h b/src/plugins/cpptools/cppmodelmanager.h index 4d44e58a0be..eeae9180c66 100644 --- a/src/plugins/cpptools/cppmodelmanager.h +++ b/src/plugins/cpptools/cppmodelmanager.h @@ -106,17 +106,13 @@ public: QFuture updateSourceFiles(const QSet &sourceFiles, ProgressNotificationMode mode = ReservedProgressNotification); - QFuture updateSourceFiles(const QFutureInterface &superFuture, - const QSet &sourceFiles, - ProgressNotificationMode mode = ReservedProgressNotification); void updateCppEditorDocuments(bool projectsUpdated = false) const; WorkingCopy workingCopy() const; QByteArray codeModelConfiguration() const; QList projectInfos() const; ProjectInfo projectInfo(ProjectExplorer::Project *project) const; - QFuture updateProjectInfo(QFutureInterface &futureInterface, - const ProjectInfo &newProjectInfo); + QFuture updateProjectInfo(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 > &futures, + void watchForCanceledProjectIndexer(const QFuture &future, ProjectExplorer::Project *project); void replaceSnapshot(const CPlusPlus::Snapshot &newSnapshot); diff --git a/src/plugins/cpptools/cppmodelmanager_test.cpp b/src/plugins/cpptools/cppmodelmanager_test.cpp index 78de5f1583e..2b77700cb84 100644 --- a/src/plugins/cpptools/cppmodelmanager_test.cpp +++ b/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 dummy; - mm->updateProjectInfo(dummy, pi); + mm->updateProjectInfo(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 dummy; - mm->updateProjectInfo(dummy, pi).waitForFinished(); + mm->updateProjectInfo(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 dummy; - mm->updateProjectInfo(dummy, pi); + mm->updateProjectInfo(pi); CPlusPlus::Snapshot snapshot; QSet refreshedFiles; @@ -387,8 +384,7 @@ void CppToolsPlugin::test_modelmanager_refresh_test_for_changes() // Reindexing triggers a reparsing thread helper.resetRefreshedSourceFiles(); - QFutureInterface dummy; - QFuture firstFuture = mm->updateProjectInfo(dummy, pi); + QFuture firstFuture = mm->updateProjectInfo(pi); QVERIFY(firstFuture.isStarted() || firstFuture.isRunning()); firstFuture.waitForFinished(); const QSet 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 subsequentFuture = mm->updateProjectInfo(dummy, pi); + QFuture subsequentFuture = mm->updateProjectInfo(pi); QVERIFY(subsequentFuture.isCanceled() && subsequentFuture.isFinished()); } diff --git a/src/plugins/cpptools/cppprojectinfogenerator.cpp b/src/plugins/cpptools/cppprojectinfogenerator.cpp index 89c7d7a2aae..7f1c332c661 100644 --- a/src/plugins/cpptools/cppprojectinfogenerator.cpp +++ b/src/plugins/cpptools/cppprojectinfogenerator.cpp @@ -42,9 +42,8 @@ using namespace ProjectExplorer; namespace CppTools { namespace Internal { -ProjectInfoGenerator::ProjectInfoGenerator( - const QFutureInterface &futureInterface, - const ProjectUpdateInfo &projectUpdateInfo) +ProjectInfoGenerator::ProjectInfoGenerator(const QFutureInterface &futureInterface, + const ProjectUpdateInfo &projectUpdateInfo) : m_futureInterface(futureInterface) , m_projectUpdateInfo(projectUpdateInfo) { diff --git a/src/plugins/cpptools/cppprojectinfogenerator.h b/src/plugins/cpptools/cppprojectinfogenerator.h index 9e484636be5..7e97f1c5ca2 100644 --- a/src/plugins/cpptools/cppprojectinfogenerator.h +++ b/src/plugins/cpptools/cppprojectinfogenerator.h @@ -36,7 +36,7 @@ namespace Internal { class ProjectInfoGenerator { public: - ProjectInfoGenerator(const QFutureInterface &futureInterface, + ProjectInfoGenerator(const QFutureInterface &futureInterface, const ProjectExplorer::ProjectUpdateInfo &projectUpdateInfo); ProjectInfo generate(); @@ -52,7 +52,7 @@ private: Utils::LanguageExtensions languageExtensions); private: - const QFutureInterface m_futureInterface; + const QFutureInterface m_futureInterface; const ProjectExplorer::ProjectUpdateInfo &m_projectUpdateInfo; bool m_cToolchainMissing = false; bool m_cxxToolchainMissing = false; diff --git a/src/plugins/cpptools/cppprojectupdater.cpp b/src/plugins/cpptools/cppprojectupdater.cpp index d17c2609302..c56b2666f0c 100644 --- a/src/plugins/cpptools/cppprojectupdater.cpp +++ b/src/plugins/cpptools/cppprojectupdater.cpp @@ -37,8 +37,10 @@ namespace CppTools { CppProjectUpdater::CppProjectUpdater() { - connect(&m_generateFutureWatcher, &QFutureWatcher::finished, - this, &CppProjectUpdater::onProjectInfoGenerated); + connect(&m_generateFutureWatcher, + &QFutureWatcher::finished, + this, + &CppProjectUpdater::onProjectInfoGenerated); } CppProjectUpdater::~CppProjectUpdater() @@ -50,7 +52,6 @@ void CppProjectUpdater::update(const ProjectExplorer::ProjectUpdateInfo &project { // Stop previous update. cancelAndWaitForFinished(); - m_futureInterface = QFutureInterface(); 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 future = Utils::runAsync([=]() { + m_generateFuture = Utils::runAsync([=](QFutureInterface &futureInterface) { ProjectUpdateInfo fullProjectUpdateInfo = projectUpdateInfo; if (fullProjectUpdateInfo.rppGenerator) fullProjectUpdateInfo.rawProjectParts = fullProjectUpdateInfo.rppGenerator(); - Internal::ProjectInfoGenerator generator(m_futureInterface, fullProjectUpdateInfo); - return generator.generate(); + Internal::ProjectInfoGenerator generator(futureInterface, fullProjectUpdateInfo); + futureInterface.reportResult(generator.generate()); }); - m_generateFutureWatcher.setFuture(future); + m_generateFutureWatcher.setFuture(m_generateFuture); } void CppProjectUpdater::cancel() { - disconnect(&m_generateFutureWatcher); - m_futureInterface.cancel(); + m_generateFutureWatcher.setFuture({}); + 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 future = CppModelManager::instance() - ->updateProjectInfo(m_futureInterface, m_generateFutureWatcher.result()); + m_updateFuture = CppModelManager::instance()->updateProjectInfo( + m_generateFutureWatcher.result()); } CppProjectUpdaterFactory::CppProjectUpdaterFactory() diff --git a/src/plugins/cpptools/cppprojectupdater.h b/src/plugins/cpptools/cppprojectupdater.h index 8167b071cf0..62e64a1daea 100644 --- a/src/plugins/cpptools/cppprojectupdater.h +++ b/src/plugins/cpptools/cppprojectupdater.h @@ -67,7 +67,8 @@ private: private: ProjectExplorer::ProjectUpdateInfo m_projectUpdateInfo; - QFutureInterface m_futureInterface; + QFuture m_generateFuture; + QFuture m_updateFuture; QFutureWatcher m_generateFutureWatcher; }; diff --git a/src/plugins/cpptools/modelmanagertesthelper.cpp b/src/plugins/cpptools/modelmanagertesthelper.cpp index 3d5f5cbdc96..08d01c02e06 100644 --- a/src/plugins/cpptools/modelmanagertesthelper.cpp +++ b/src/plugins/cpptools/modelmanagertesthelper.cpp @@ -93,8 +93,7 @@ ModelManagerTestHelper::Project *ModelManagerTestHelper::createProject(const QSt QSet ModelManagerTestHelper::updateProjectInfo(const CppTools::ProjectInfo &projectInfo) { resetRefreshedSourceFiles(); - QFutureInterface dummy; - CppModelManager::instance()->updateProjectInfo(dummy, projectInfo).waitForFinished(); + CppModelManager::instance()->updateProjectInfo(projectInfo).waitForFinished(); QCoreApplication::processEvents(); return waitForRefreshedSourceFiles(); } diff --git a/tests/unit/unittest/cppprojectinfogenerator-test.cpp b/tests/unit/unittest/cppprojectinfogenerator-test.cpp index ceb3c8b8aae..98eb8feeaa3 100644 --- a/tests/unit/unittest/cppprojectinfogenerator-test.cpp +++ b/tests/unit/unittest/cppprojectinfogenerator-test.cpp @@ -163,7 +163,7 @@ void ProjectInfoGenerator::SetUp() ProjectInfo ProjectInfoGenerator::generate() { - QFutureInterface fi; + QFutureInterface fi; ProjectExplorer::ConcreteToolChain aToolChain; projectUpdateInfo.rawProjectParts += rawProjectPart;