From 2948d282ce6518d1befd3e1d3371089a9863cac8 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Thu, 14 Oct 2021 10:27:45 +0200 Subject: [PATCH] Don't call DocumentManager::addDocument() from non-main thread Detect that constructor of ResourceTopLevelNode is being run from non-main thread and omit creation of ResourceFileWatcher in this case. The construction of ResourceFileWatcher will be postponed until the node tree returns back to the main thread. This happens later inside Project::setRootProjectNode() when ProjectTree::applyTreeManager() is called for the second time - this time it's done from the main thread. In order to setup the lacking resource file watchers we install an additional folder node visitor only in case when the handler is called from main thread. The visitor sets up the lacking resource watchers if that's still needed. Amends: 0bcab32657f1511892eda53194dce259e40edf21 Fixes: QTCREATORBUG-26417 Change-Id: Ia1bfb7f284afb833b6b4291accc4d0a91bd0d6c5 Reviewed-by: Eike Ziller --- src/libs/utils/CMakeLists.txt | 1 + src/libs/utils/threadutils.cpp | 38 ++++++++++++++++ src/libs/utils/threadutils.h | 35 +++++++++++++++ src/libs/utils/utils-lib.pri | 2 + src/libs/utils/utils.qbs | 2 + .../fileapidataextractor.cpp | 2 +- src/plugins/coreplugin/documentmanager.cpp | 3 ++ src/plugins/projectexplorer/project.cpp | 3 +- src/plugins/projectexplorer/projecttree.cpp | 4 +- src/plugins/projectexplorer/projecttree.h | 9 +++- src/plugins/projectexplorer/treescanner.cpp | 2 +- .../qbsprojectmanager/qbsnodetreebuilder.cpp | 2 +- .../resourceeditor/resourceeditorplugin.cpp | 43 ++++++++++++------- src/plugins/resourceeditor/resourcenode.cpp | 16 +++++-- src/plugins/resourceeditor/resourcenode.h | 1 + 15 files changed, 136 insertions(+), 27 deletions(-) create mode 100644 src/libs/utils/threadutils.cpp create mode 100644 src/libs/utils/threadutils.h diff --git a/src/libs/utils/CMakeLists.txt b/src/libs/utils/CMakeLists.txt index 9db6ea5f1e7..b94b2b42227 100644 --- a/src/libs/utils/CMakeLists.txt +++ b/src/libs/utils/CMakeLists.txt @@ -168,6 +168,7 @@ add_qtc_library(Utils textfileformat.cpp textfileformat.h textutils.cpp textutils.h theme/theme.cpp theme/theme.h theme/theme_p.h + threadutils.cpp threadutils.h tooltip/effects.h tooltip/tips.cpp tooltip/tips.h tooltip/tooltip.cpp tooltip/tooltip.h diff --git a/src/libs/utils/threadutils.cpp b/src/libs/utils/threadutils.cpp new file mode 100644 index 00000000000..7d10199c4ab --- /dev/null +++ b/src/libs/utils/threadutils.cpp @@ -0,0 +1,38 @@ +/**************************************************************************** +** +** Copyright (C) 2021 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of Qt Creator. +** +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-3.0.html. +** +****************************************************************************/ + +#include "threadutils.h" + +#include +#include + +namespace Utils { + +bool isMainThread() +{ + return QThread::currentThread() == qApp->thread(); +} + +} // namespace Utils diff --git a/src/libs/utils/threadutils.h b/src/libs/utils/threadutils.h new file mode 100644 index 00000000000..cc59658c277 --- /dev/null +++ b/src/libs/utils/threadutils.h @@ -0,0 +1,35 @@ +/**************************************************************************** +** +** Copyright (C) 2021 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of Qt Creator. +** +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-3.0.html. +** +****************************************************************************/ + +#pragma once + +#include "utils_global.h" + +namespace Utils { + +QTCREATOR_UTILS_EXPORT bool isMainThread(); + +} // namespace Utils + diff --git a/src/libs/utils/utils-lib.pri b/src/libs/utils/utils-lib.pri index ddaae4bb697..a8280ba3267 100644 --- a/src/libs/utils/utils-lib.pri +++ b/src/libs/utils/utils-lib.pri @@ -143,6 +143,7 @@ SOURCES += \ $$PWD/link.cpp \ $$PWD/linecolumn.cpp \ $$PWD/multitextcursor.cpp \ + $$PWD/threadutils.cpp \ $$PWD/singleton.cpp HEADERS += \ @@ -310,6 +311,7 @@ HEADERS += \ $$PWD/launchersocket.h \ $$PWD/qtcsettings.h \ $$PWD/multitextcursor.h \ + $$PWD/threadutils.h \ $$PWD/singleton.h FORMS += $$PWD/filewizardpage.ui \ diff --git a/src/libs/utils/utils.qbs b/src/libs/utils/utils.qbs index 77f80dd81b8..6d888f87ac4 100644 --- a/src/libs/utils/utils.qbs +++ b/src/libs/utils/utils.qbs @@ -298,6 +298,8 @@ Project { "textfileformat.h", "textutils.cpp", "textutils.h", + "threadutils.cpp", + "threadutils.h", "treemodel.cpp", "treemodel.h", "treeviewcombobox.cpp", diff --git a/src/plugins/cmakeprojectmanager/fileapidataextractor.cpp b/src/plugins/cmakeprojectmanager/fileapidataextractor.cpp index 7200a060002..d5836605f48 100644 --- a/src/plugins/cmakeprojectmanager/fileapidataextractor.cpp +++ b/src/plugins/cmakeprojectmanager/fileapidataextractor.cpp @@ -757,7 +757,7 @@ FileApiQtcData extractData(FileApiData &input, result.projectParts = generateRawProjectParts(data, sourceDirectory, buildDirectory); auto rootProjectNode = generateRootProjectNode(data, sourceDirectory, buildDirectory); - ProjectTree::applyTreeManager(rootProjectNode.get()); // QRC nodes + ProjectTree::applyTreeManager(rootProjectNode.get(), ProjectTree::AsyncPhase); // QRC nodes result.rootProjectNode = std::move(rootProjectNode); setupLocationInfoForTargets(result.rootProjectNode.get(), result.buildTargets); diff --git a/src/plugins/coreplugin/documentmanager.cpp b/src/plugins/coreplugin/documentmanager.cpp index d7d5f17d8f1..4fd4213dbbb 100644 --- a/src/plugins/coreplugin/documentmanager.cpp +++ b/src/plugins/coreplugin/documentmanager.cpp @@ -55,6 +55,7 @@ #include #include #include +#include #include #include @@ -326,6 +327,7 @@ static void addFileInfo(IDocument *document, const FilePath &filePath, const Fil (The added file names are guaranteed to be absolute and cleaned.) */ static void addFileInfos(const QList &documents) { + QTC_ASSERT(isMainThread(), return); FilePaths pathsToWatch; FilePaths linkPathsToWatch; for (IDocument *document : documents) { @@ -400,6 +402,7 @@ void DocumentManager::addDocuments(const QList &documents, bool add */ static void removeFileInfo(IDocument *document) { + QTC_ASSERT(isMainThread(), return); if (!d->m_documentsWithWatch.contains(document)) return; foreach (const FilePath &filePath, d->m_documentsWithWatch.value(document)) { diff --git a/src/plugins/projectexplorer/project.cpp b/src/plugins/projectexplorer/project.cpp index 620eb61f5fa..38cdf161713 100644 --- a/src/plugins/projectexplorer/project.cpp +++ b/src/plugins/projectexplorer/project.cpp @@ -586,7 +586,8 @@ void Project::setRootProjectNode(std::unique_ptr &&root) } if (root) { - ProjectTree::applyTreeManager(root.get()); + ProjectTree::applyTreeManager(root.get(), ProjectTree::AsyncPhase); + ProjectTree::applyTreeManager(root.get(), ProjectTree::FinalPhase); root->setParentFolderNode(d->m_containerNode.get()); } diff --git a/src/plugins/projectexplorer/projecttree.cpp b/src/plugins/projectexplorer/projecttree.cpp index aa8e79f9efb..ee040d5a37f 100644 --- a/src/plugins/projectexplorer/projecttree.cpp +++ b/src/plugins/projectexplorer/projecttree.cpp @@ -402,13 +402,13 @@ void ProjectTree::registerTreeManager(const TreeManagerFunction &treeChange) s_instance->m_treeManagers.append(treeChange); } -void ProjectTree::applyTreeManager(FolderNode *folder) +void ProjectTree::applyTreeManager(FolderNode *folder, ConstructionPhase phase) { if (!folder) return; for (TreeManagerFunction &f : s_instance->m_treeManagers) - f(folder); + f(folder, phase); } bool ProjectTree::hasNode(const Node *node) diff --git a/src/plugins/projectexplorer/projecttree.h b/src/plugins/projectexplorer/projecttree.h index 060dad22903..3947666d3a6 100644 --- a/src/plugins/projectexplorer/projecttree.h +++ b/src/plugins/projectexplorer/projecttree.h @@ -68,6 +68,11 @@ public: const bool m_active = false; }; + enum ConstructionPhase { + AsyncPhase, + FinalPhase + }; + // Integration with ProjectTreeWidget static void registerWidget(Internal::ProjectTreeWidget *widget); static void unregisterWidget(Internal::ProjectTreeWidget *widget); @@ -79,9 +84,9 @@ public: static void highlightProject(Project *project, const QString &message); - using TreeManagerFunction = std::function; + using TreeManagerFunction = std::function; static void registerTreeManager(const TreeManagerFunction &treeChange); - static void applyTreeManager(FolderNode *folder); + static void applyTreeManager(FolderNode *folder, ConstructionPhase phase); // Nodes: static bool hasNode(const Node *node); diff --git a/src/plugins/projectexplorer/treescanner.cpp b/src/plugins/projectexplorer/treescanner.cpp index ce6da40b1c2..88e7b8d4a83 100644 --- a/src/plugins/projectexplorer/treescanner.cpp +++ b/src/plugins/projectexplorer/treescanner.cpp @@ -159,7 +159,7 @@ static std::unique_ptr createFolderNode(const Utils::FilePath &direc std::unique_ptr node(fn->clone()); fileSystemNode->addNestedNode(std::move(node)); } - ProjectTree::applyTreeManager(fileSystemNode.get()); // QRC nodes + ProjectTree::applyTreeManager(fileSystemNode.get(), ProjectTree::AsyncPhase); // QRC nodes return fileSystemNode; } diff --git a/src/plugins/qbsprojectmanager/qbsnodetreebuilder.cpp b/src/plugins/qbsprojectmanager/qbsnodetreebuilder.cpp index c481975753f..7e3c31c378a 100644 --- a/src/plugins/qbsprojectmanager/qbsnodetreebuilder.cpp +++ b/src/plugins/qbsprojectmanager/qbsnodetreebuilder.cpp @@ -232,7 +232,7 @@ QbsProjectNode *QbsNodeTreeBuilder::buildTree(const QString &projectName, } buildSystemFiles->compress(); root->addNode(std::move(buildSystemFiles)); - ProjectTree::applyTreeManager(root.get()); // QRC nodes + ProjectTree::applyTreeManager(root.get(), ProjectTree::AsyncPhase); // QRC nodes return root.release(); } diff --git a/src/plugins/resourceeditor/resourceeditorplugin.cpp b/src/plugins/resourceeditor/resourceeditorplugin.cpp index 62a4d752456..f901a417dff 100644 --- a/src/plugins/resourceeditor/resourceeditorplugin.cpp +++ b/src/plugins/resourceeditor/resourceeditorplugin.cpp @@ -46,6 +46,7 @@ #include #include #include +#include #include #include @@ -247,21 +248,33 @@ ResourceEditorPluginPrivate::ResourceEditorPluginPrivate(ResourceEditorPlugin *q void ResourceEditorPlugin::extensionsInitialized() { - ProjectTree::registerTreeManager([](FolderNode *folder) { - QList toReplace; - folder->forEachNode([&toReplace](FileNode *fn) { - if (fn->fileType() == FileType::Resource) - toReplace.append(fn); - }); - - for (FileNode *file : qAsConst(toReplace)) { - FolderNode *const pn = file->parentFolderNode(); - QTC_ASSERT(pn, continue); - const Utils::FilePath path = file->filePath(); - auto topLevel = std::make_unique(path, pn->filePath()); - topLevel->setEnabled(file->isEnabled()); - topLevel->setIsGenerated(file->isGenerated()); - pn->replaceSubtree(file, std::move(topLevel)); + ProjectTree::registerTreeManager([](FolderNode *folder, ProjectTree::ConstructionPhase phase) { + switch (phase) { + case ProjectTree::AsyncPhase: { + QList toReplace; + folder->forEachNode([&toReplace](FileNode *fn) { + if (fn->fileType() == FileType::Resource) + toReplace.append(fn); + }); + for (FileNode *file : qAsConst(toReplace)) { + FolderNode *const pn = file->parentFolderNode(); + QTC_ASSERT(pn, continue); + const Utils::FilePath path = file->filePath(); + auto topLevel = std::make_unique(path, pn->filePath()); + topLevel->setEnabled(file->isEnabled()); + topLevel->setIsGenerated(file->isGenerated()); + pn->replaceSubtree(file, std::move(topLevel)); + } + break; + } + case ProjectTree::FinalPhase: { + folder->forEachNode({}, [](FolderNode *fn) { + auto *topLevel = dynamic_cast(fn); + if (topLevel) + topLevel->setupWatcherIfNeeded(); + }); + break; + } } }); } diff --git a/src/plugins/resourceeditor/resourcenode.cpp b/src/plugins/resourceeditor/resourcenode.cpp index 30be6b0c0a3..75f35da4d8a 100644 --- a/src/plugins/resourceeditor/resourcenode.cpp +++ b/src/plugins/resourceeditor/resourcenode.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include @@ -251,10 +252,8 @@ ResourceTopLevelNode::ResourceTopLevelNode(const FilePath &filePath, setShowWhenEmpty(true); if (!filePath.isEmpty()) { - if (filePath.isReadableFile()) { - m_document = new ResourceFileWatcher(this); - DocumentManager::addDocument(m_document); - } + if (filePath.isReadableFile()) + setupWatcherIfNeeded(); } else { m_contents = contents; } @@ -267,6 +266,15 @@ ResourceTopLevelNode::ResourceTopLevelNode(const FilePath &filePath, addInternalNodes(); } +void ResourceTopLevelNode::setupWatcherIfNeeded() +{ + if (m_document || !isMainThread()) + return; + + m_document = new ResourceFileWatcher(this); + DocumentManager::addDocument(m_document); +} + ResourceTopLevelNode::~ResourceTopLevelNode() { if (m_document) diff --git a/src/plugins/resourceeditor/resourcenode.h b/src/plugins/resourceeditor/resourcenode.h index a992e267dc8..03ae8039f0c 100644 --- a/src/plugins/resourceeditor/resourcenode.h +++ b/src/plugins/resourceeditor/resourcenode.h @@ -39,6 +39,7 @@ public: const QString &contents = {}); ~ResourceTopLevelNode() override; + void setupWatcherIfNeeded(); void addInternalNodes(); bool supportsAction(ProjectExplorer::ProjectAction action, const Node *node) const override;