CMakeFileCompletionAssist: Fix a crash on completion

Avoid calling not-thread safe functions from inside
the CMakeFileCompletionAssist::performAsync().

Move calling these functions before the asyncRun, collect
the needed data inside the PerformInputData structure
and pass it to the other thread instead.

Fixes: QTCREATORBUG-29683
Change-Id: I56127163a47339bc45d47f3a7d2c76d518b640f7
Reviewed-by: Cristian Adam <cristian.adam@qt.io>
This commit is contained in:
Jarek Kobus
2023-10-02 17:19:09 +02:00
parent 25feb7db89
commit 07e758147b
2 changed files with 75 additions and 48 deletions

View File

@@ -23,6 +23,7 @@
#include <texteditor/codeassist/genericproposal.h>
#include <texteditor/texteditorsettings.h>
#include <utils/async.h>
#include <utils/fsengine/fileiconprovider.h>
#include <utils/utilsicons.h>
@@ -32,12 +33,15 @@ using namespace Utils;
namespace CMakeProjectManager::Internal {
class PerformInputData;
class CMakeFileCompletionAssist : public AsyncProcessor
{
public:
CMakeFileCompletionAssist();
IAssistProposal *performAsync() final;
IAssistProposal *perform() final;
IAssistProposal *performAsync() final { return nullptr; }
const QIcon m_variableIcon;
const QIcon m_projectVariableIcon;
@@ -51,6 +55,10 @@ public:
const QIcon m_importedTargetIcon;
TextEditor::SnippetAssistCollector m_snippetCollector;
private:
IAssistProposal *doPerform(const PerformInputData &data);
PerformInputData generatePerformInputData() const;
};
CMakeFileCompletionAssist::CMakeFileCompletionAssist()
@@ -242,41 +250,62 @@ static QPair<QStringList, QStringList> getLocalFunctionsAndVariables(const QByte
return {functions, variables};
}
IAssistProposal *CMakeFileCompletionAssist::performAsync()
class PerformInputData
{
public:
CMakeKeywords keywords;
CMakeKeywords projectKeywords;
const FilePath &filePath = interface()->filePath();
if (!filePath.isEmpty() && filePath.isFile()) {
if (auto tool = CMakeToolManager::defaultProjectOrDefaultCMakeTool())
keywords = tool->keywords();
}
QStringList buildTargets;
QStringList importedTargets;
QStringList findPackageVariables;
};
PerformInputData CMakeFileCompletionAssist::generatePerformInputData() const
{
PerformInputData data;
const FilePath &filePath = interface()->filePath();
if (!filePath.isEmpty() && filePath.isFile()) {
if (auto tool = CMakeToolManager::defaultProjectOrDefaultCMakeTool())
data.keywords = tool->keywords();
}
if (auto bs = qobject_cast<CMakeBuildSystem *>(ProjectTree::currentBuildSystem())) {
for (const auto &target : std::as_const(bs->buildTargets()))
if (target.targetType != TargetType::UtilityType)
buildTargets << target.title;
projectKeywords = bs->projectKeywords();
importedTargets = bs->projectImportedTargets();
findPackageVariables = bs->projectFindPackageVariables();
data.buildTargets << target.title;
data.projectKeywords = bs->projectKeywords();
data.importedTargets = bs->projectImportedTargets();
data.findPackageVariables = bs->projectFindPackageVariables();
}
return data;
}
IAssistProposal *CMakeFileCompletionAssist::perform()
{
IAssistProposal *result = immediateProposal();
interface()->prepareForAsyncUse();
m_watcher.setFuture(Utils::asyncRun([this, inputData = generatePerformInputData()] {
interface()->recreateTextDocument();
return doPerform(inputData);
}));
return result;
}
IAssistProposal *CMakeFileCompletionAssist::doPerform(const PerformInputData &data)
{
if (isInComment(interface()))
return nullptr;
const int startPos = findWordStart(interface(), interface()->position());
const int functionStart = findFunctionStart(interface());
const int prevFunctionEnd = findFunctionEnd(interface());
QString functionName;
if (functionStart > prevFunctionEnd) {
int functionStartPos = findWordStart(interface(), functionStart);
functionName
= interface()->textAt(functionStartPos, functionStart - functionStartPos);
const int functionStartPos = findWordStart(interface(), functionStart);
functionName = interface()->textAt(functionStartPos, functionStart - functionStartPos);
}
if (interface()->reason() == IdleEditor) {
@@ -296,12 +325,12 @@ IAssistProposal *CMakeFileCompletionAssist::performAsync()
const QString varGenexToken = interface()->textAt(startPos - 2, 2);
if (varGenexToken == "${" || varGenexToken == "$<") {
if (varGenexToken == "${") {
items.append(generateList(keywords.variables, m_variableIcon));
items.append(generateList(projectKeywords.variables, m_projectVariableIcon));
items.append(generateList(findPackageVariables, m_projectVariableIcon));
items.append(generateList(data.keywords.variables, m_variableIcon));
items.append(generateList(data.projectKeywords.variables, m_projectVariableIcon));
items.append(generateList(data.findPackageVariables, m_projectVariableIcon));
}
if (varGenexToken == "$<")
items.append(generateList(keywords.generatorExpressions, m_genexIcon));
items.append(generateList(data.keywords.generatorExpressions, m_genexIcon));
return new GenericProposal(startPos, items);
}
@@ -312,56 +341,56 @@ IAssistProposal *CMakeFileCompletionAssist::performAsync()
if (functionName == "if" || functionName == "elseif" || functionName == "while"
|| functionName == "set" || functionName == "list"
|| functionName == "cmake_print_variables") {
items.append(generateList(keywords.variables, m_variableIcon));
items.append(generateList(projectKeywords.variables, m_projectVariableIcon));
items.append(generateList(findPackageVariables, m_projectVariableIcon));
items.append(generateList(data.keywords.variables, m_variableIcon));
items.append(generateList(data.projectKeywords.variables, m_projectVariableIcon));
items.append(generateList(data.findPackageVariables, m_projectVariableIcon));
items.append(generateList(localVariables, m_variableIcon));
}
if (functionName == "if" || functionName == "elseif" || functionName == "cmake_policy")
items.append(generateList(keywords.policies, m_variableIcon));
items.append(generateList(data.keywords.policies, m_variableIcon));
if (functionName.contains("path") || functionName.contains("file")
|| functionName.contains("add_executable") || functionName.contains("add_library")
|| functionName == "include" || functionName == "add_subdirectory"
|| functionName == "install" || functionName == "target_sources" || functionName == "set"
|| functionName == "list") {
|| functionName == "install" || functionName == "target_sources"
|| functionName == "set" || functionName == "list") {
fileStartPos = addFilePathItems(interface(), items, startPos);
}
if (functionName == "set_property" || functionName == "cmake_print_properties")
items.append(generateList(keywords.properties, m_propertyIcon));
items.append(generateList(data.keywords.properties, m_propertyIcon));
if (functionName == "set_directory_properties")
items.append(generateList(keywords.directoryProperties, m_propertyIcon));
items.append(generateList(data.keywords.directoryProperties, m_propertyIcon));
if (functionName == "set_source_files_properties")
items.append(generateList(keywords.sourceProperties, m_propertyIcon));
items.append(generateList(data.keywords.sourceProperties, m_propertyIcon));
if (functionName == "set_target_properties")
items.append(generateList(keywords.targetProperties, m_propertyIcon));
items.append(generateList(data.keywords.targetProperties, m_propertyIcon));
if (functionName == "set_tests_properties")
items.append(generateList(keywords.testProperties, m_propertyIcon));
items.append(generateList(data.keywords.testProperties, m_propertyIcon));
if (functionName == "include" && !onlyFileItems())
items.append(generateList(keywords.includeStandardModules, m_moduleIcon));
items.append(generateList(data.keywords.includeStandardModules, m_moduleIcon));
if (functionName == "find_package")
items.append(generateList(keywords.findModules, m_moduleIcon));
items.append(generateList(data.keywords.findModules, m_moduleIcon));
if ((functionName.contains("target") || functionName == "install"
|| functionName == "add_dependencies" || functionName == "set_property"
|| functionName == "export" || functionName == "cmake_print_properties"
|| functionName == "if" || functionName == "elseif")
&& !onlyFileItems()) {
items.append(generateList(buildTargets, m_targetsIcon));
items.append(generateList(importedTargets, m_importedTargetIcon));
items.append(generateList(data.buildTargets, m_targetsIcon));
items.append(generateList(data.importedTargets, m_importedTargetIcon));
}
if (keywords.functionArgs.contains(functionName) && !onlyFileItems()) {
QStringList functionSymbols = keywords.functionArgs.value(functionName);
if (data.keywords.functionArgs.contains(functionName) && !onlyFileItems()) {
const QStringList functionSymbols = data.keywords.functionArgs.value(functionName);
items.append(generateList(functionSymbols, m_argsIcon));
} else if (functionName.isEmpty()) {
// On a new line we just want functions
items.append(generateList(keywords.functions, m_functionIcon));
items.append(generateList(projectKeywords.functions, m_projectFunctionIcon));
items.append(generateList(data.keywords.functions, m_functionIcon));
items.append(generateList(data.projectKeywords.functions, m_projectFunctionIcon));
items.append(generateList(localFunctions, m_functionIcon));
// Snippets would make more sense only for the top level suggestions
@@ -370,14 +399,14 @@ IAssistProposal *CMakeFileCompletionAssist::performAsync()
// Inside an unknown function we could have variables or properties
fileStartPos = addFilePathItems(interface(), items, startPos);
if (!onlyFileItems()) {
items.append(generateList(keywords.variables, m_variableIcon));
items.append(generateList(projectKeywords.variables, m_projectVariableIcon));
items.append(generateList(data.keywords.variables, m_variableIcon));
items.append(generateList(data.projectKeywords.variables, m_projectVariableIcon));
items.append(generateList(localVariables, m_variableIcon));
items.append(generateList(findPackageVariables, m_projectVariableIcon));
items.append(generateList(data.findPackageVariables, m_projectVariableIcon));
items.append(generateList(keywords.properties, m_propertyIcon));
items.append(generateList(buildTargets, m_targetsIcon));
items.append(generateList(importedTargets, m_importedTargetIcon));
items.append(generateList(data.keywords.properties, m_propertyIcon));
items.append(generateList(data.buildTargets, m_targetsIcon));
items.append(generateList(data.importedTargets, m_importedTargetIcon));
}
}