diff options
author | Nikolai Kosjar <nikolai.kosjar@theqtcompany.com> | 2015-09-01 17:34:07 +0200 |
---|---|---|
committer | Nikolai Kosjar <nikolai.kosjar@theqtcompany.com> | 2015-09-02 12:37:23 +0000 |
commit | 169556db2f63912b7eaa2375dbc17c52ffd57586 (patch) | |
tree | c2a4a979dfbdabc6c2d93a24e038a15b19f8bab5 | |
parent | c504e56d0c7fafba4f8c15997e3927b5ca02adc5 (diff) | |
download | qt-creator-169556db2f63912b7eaa2375dbc17c52ffd57586.tar.gz |
C++: Fix crash after triggering completion and closing editor
Fix use-after-free for the following case:
1. Open an editor
2. Trigger a long processing completion
(e.g. simulate with QThread::msleep in
CppCompletionAssistInterface::getCppSpecifics)
3. ...and immediately close the editor (e.g. with Ctrl+W)
4. Wait until it crashes.
The completion thread relied on the BuiltinEditorDocumentParser object,
which is deleted once the editor is closed. Fixed by sharing the
ownership of that object between the *EditorDocumentProcessor and the
completion assist interface.
This case came up when doing tests for the bug report below.
Task-number: QTCREATORBUG-14991
Change-Id: I0b009229e68fc6b7838740858cdc41a32403fe6f
Reviewed-by: David Schulz <david.schulz@theqtcompany.com>
17 files changed, 49 insertions, 50 deletions
diff --git a/src/plugins/clangcodemodel/clangeditordocumentprocessor.cpp b/src/plugins/clangcodemodel/clangeditordocumentprocessor.cpp index f8a4191513..92763ba9d3 100644 --- a/src/plugins/clangcodemodel/clangeditordocumentprocessor.cpp +++ b/src/plugins/clangcodemodel/clangeditordocumentprocessor.cpp @@ -94,7 +94,7 @@ ClangEditorDocumentProcessor::ClangEditorDocumentProcessor( TextEditor::TextDocument *document) : BaseEditorDocumentProcessor(document) , m_modelManagerSupport(modelManagerSupport) - , m_parser(document->filePath().toString()) + , m_parser(new ClangEditorDocumentParser(document->filePath().toString())) , m_parserRevision(0) , m_semanticHighlighter(document) , m_builtinProcessor(document, /*enableSemanticHighlighter=*/ false) @@ -114,7 +114,7 @@ ClangEditorDocumentProcessor::ClangEditorDocumentProcessor( const int firstLine = 1; const int lastLine = baseTextDocument()->document()->blockCount(); - CreateMarkers *createMarkers = CreateMarkers::create(m_parser.semanticMarker(), + CreateMarkers *createMarkers = CreateMarkers::create(m_parser->semanticMarker(), baseTextDocument()->filePath().toString(), firstLine, lastLine); return createMarkers->start(); @@ -169,9 +169,9 @@ CppTools::SemanticInfo ClangEditorDocumentProcessor::recalculateSemanticInfo() return m_builtinProcessor.recalculateSemanticInfo(); } -CppTools::BaseEditorDocumentParser *ClangEditorDocumentProcessor::parser() +CppTools::BaseEditorDocumentParser::Ptr ClangEditorDocumentProcessor::parser() { - return &m_parser; + return m_parser; } CPlusPlus::Snapshot ClangEditorDocumentProcessor::snapshot() @@ -196,7 +196,7 @@ ClangEditorDocumentProcessor *ClangEditorDocumentProcessor::get(const QString &f void ClangEditorDocumentProcessor::updateProjectPartAndTranslationUnitForCompletion() { - const CppTools::ProjectPart::Ptr projectPart = m_parser.projectPart(); + const CppTools::ProjectPart::Ptr projectPart = m_parser->projectPart(); QTC_ASSERT(projectPart, return); updateTranslationUnitForCompletion(*projectPart.data()); @@ -209,11 +209,11 @@ void ClangEditorDocumentProcessor::onParserFinished() return; // Emit ifdefed out blocks - const auto ifdefoutBlocks = toTextEditorBlocks(m_parser.ifdefedOutBlocks()); + const auto ifdefoutBlocks = toTextEditorBlocks(m_parser->ifdefedOutBlocks()); emit ifdefedOutBlocksUpdated(revision(), ifdefoutBlocks); // Emit code warnings - const auto diagnostics = toCppToolsDiagnostics(filePath(), m_parser.diagnostics()); + const auto diagnostics = toCppToolsDiagnostics(filePath(), m_parser->diagnostics()); const auto codeWarnings = toTextEditorSelections(diagnostics, textDocument()); emit codeWarningsUpdated(revision(), codeWarnings); diff --git a/src/plugins/clangcodemodel/clangeditordocumentprocessor.h b/src/plugins/clangcodemodel/clangeditordocumentprocessor.h index 4d8564685c..2cae3a4d4e 100644 --- a/src/plugins/clangcodemodel/clangeditordocumentprocessor.h +++ b/src/plugins/clangcodemodel/clangeditordocumentprocessor.h @@ -59,7 +59,7 @@ public: void semanticRehighlight() override; void recalculateSemanticInfoDetached(bool force) override; CppTools::SemanticInfo recalculateSemanticInfo() override; - CppTools::BaseEditorDocumentParser *parser() override; + CppTools::BaseEditorDocumentParser::Ptr parser() override; CPlusPlus::Snapshot snapshot() override; bool isParserRunning() const override; @@ -78,7 +78,7 @@ private: QPointer<ModelManagerSupportClang> m_modelManagerSupport; - ClangEditorDocumentParser m_parser; + QSharedPointer<ClangEditorDocumentParser> m_parser; CppTools::ProjectPart::Ptr m_projectPart; QFutureWatcher<void> m_parserWatcher; unsigned m_parserRevision; diff --git a/src/plugins/clangcodemodel/clangutils.cpp b/src/plugins/clangcodemodel/clangutils.cpp index 9ceb6a208d..63707608a5 100644 --- a/src/plugins/clangcodemodel/clangutils.cpp +++ b/src/plugins/clangcodemodel/clangutils.cpp @@ -234,7 +234,7 @@ QStringList createPCHInclusionOptions(const QString &pchFile) ProjectPart::Ptr projectPartForFile(const QString &filePath) { - if (CppTools::BaseEditorDocumentParser *parser = CppTools::BaseEditorDocumentParser::get(filePath)) + if (const auto parser = CppTools::BaseEditorDocumentParser::get(filePath)) return parser->projectPart(); return ProjectPart::Ptr(); } diff --git a/src/plugins/cppeditor/cppeditordocument.cpp b/src/plugins/cppeditor/cppeditordocument.cpp index e64e4fe646..8db9c73156 100644 --- a/src/plugins/cppeditor/cppeditordocument.cpp +++ b/src/plugins/cppeditor/cppeditordocument.cpp @@ -266,7 +266,7 @@ void CppEditorDocument::updatePreprocessorSettings() void CppEditorDocument::setPreprocessorSettings(const CppTools::ProjectPart::Ptr &projectPart, const QByteArray &defines) { - CppTools::BaseEditorDocumentParser *parser = processor()->parser(); + const auto parser = processor()->parser(); QTC_ASSERT(parser, return); if (parser->projectPart() != projectPart || parser->configuration().editorDefines != defines) { CppTools::BaseEditorDocumentParser::Configuration config = parser->configuration(); diff --git a/src/plugins/cpptools/baseeditordocumentparser.cpp b/src/plugins/cpptools/baseeditordocumentparser.cpp index 314b7f98ab..006c998472 100644 --- a/src/plugins/cpptools/baseeditordocumentparser.cpp +++ b/src/plugins/cpptools/baseeditordocumentparser.cpp @@ -104,14 +104,14 @@ ProjectPart::Ptr BaseEditorDocumentParser::projectPart() const return state().projectPart; } -BaseEditorDocumentParser *BaseEditorDocumentParser::get(const QString &filePath) +BaseEditorDocumentParser::Ptr BaseEditorDocumentParser::get(const QString &filePath) { CppModelManager *cmmi = CppModelManager::instance(); if (CppEditorDocumentHandle *cppEditorDocument = cmmi->cppEditorDocument(filePath)) { if (BaseEditorDocumentProcessor *processor = cppEditorDocument->processor()) return processor->parser(); } - return 0; + return BaseEditorDocumentParser::Ptr(); } ProjectPart::Ptr BaseEditorDocumentParser::determineProjectPart(const QString &filePath, diff --git a/src/plugins/cpptools/baseeditordocumentparser.h b/src/plugins/cpptools/baseeditordocumentparser.h index 05f210716d..d1d0608ec3 100644 --- a/src/plugins/cpptools/baseeditordocumentparser.h +++ b/src/plugins/cpptools/baseeditordocumentparser.h @@ -44,7 +44,8 @@ class CPPTOOLS_EXPORT BaseEditorDocumentParser : public QObject Q_OBJECT public: - static BaseEditorDocumentParser *get(const QString &filePath); + using Ptr = QSharedPointer<BaseEditorDocumentParser>;; + static Ptr get(const QString &filePath); struct Configuration { bool stickToPreviousProjectPart = true; diff --git a/src/plugins/cpptools/baseeditordocumentprocessor.cpp b/src/plugins/cpptools/baseeditordocumentprocessor.cpp index d627ab3cb7..5516d6c219 100644 --- a/src/plugins/cpptools/baseeditordocumentprocessor.cpp +++ b/src/plugins/cpptools/baseeditordocumentprocessor.cpp @@ -118,7 +118,7 @@ QList<QTextEdit::ExtraSelection> BaseEditorDocumentProcessor::toTextEditorSelect } void BaseEditorDocumentProcessor::runParser(QFutureInterface<void> &future, - BaseEditorDocumentParser *parser, + BaseEditorDocumentParser::Ptr parser, BaseEditorDocumentParser::InMemoryInfo info) { future.setProgressRange(0, 1); diff --git a/src/plugins/cpptools/baseeditordocumentprocessor.h b/src/plugins/cpptools/baseeditordocumentprocessor.h index 76c6c057ea..59216d2f0c 100644 --- a/src/plugins/cpptools/baseeditordocumentprocessor.h +++ b/src/plugins/cpptools/baseeditordocumentprocessor.h @@ -62,7 +62,7 @@ public: virtual void recalculateSemanticInfoDetached(bool force) = 0; virtual CppTools::SemanticInfo recalculateSemanticInfo() = 0; virtual CPlusPlus::Snapshot snapshot() = 0; - virtual BaseEditorDocumentParser *parser() = 0; + virtual BaseEditorDocumentParser::Ptr parser() = 0; virtual bool isParserRunning() const = 0; public: @@ -85,7 +85,7 @@ protected: QTextDocument *textDocument); static void runParser(QFutureInterface<void> &future, - CppTools::BaseEditorDocumentParser *parser, + BaseEditorDocumentParser::Ptr parser, BaseEditorDocumentParser::InMemoryInfo info); // Convenience diff --git a/src/plugins/cpptools/builtineditordocumentparser.cpp b/src/plugins/cpptools/builtineditordocumentparser.cpp index 41035333f6..2331ff2887 100644 --- a/src/plugins/cpptools/builtineditordocumentparser.cpp +++ b/src/plugins/cpptools/builtineditordocumentparser.cpp @@ -226,11 +226,11 @@ ProjectPart::HeaderPaths BuiltinEditorDocumentParser::headerPaths() const return extraState().headerPaths; } -BuiltinEditorDocumentParser *BuiltinEditorDocumentParser::get(const QString &filePath) +BuiltinEditorDocumentParser::Ptr BuiltinEditorDocumentParser::get(const QString &filePath) { - if (BaseEditorDocumentParser *b = BaseEditorDocumentParser::get(filePath)) - return qobject_cast<BuiltinEditorDocumentParser *>(b); - return 0; + if (BaseEditorDocumentParser::Ptr b = BaseEditorDocumentParser::get(filePath)) + return b.objectCast<BuiltinEditorDocumentParser>(); + return BuiltinEditorDocumentParser::Ptr(); } void BuiltinEditorDocumentParser::addFileAndDependencies(Snapshot *snapshot, diff --git a/src/plugins/cpptools/builtineditordocumentparser.h b/src/plugins/cpptools/builtineditordocumentparser.h index c27b336622..9fd198024c 100644 --- a/src/plugins/cpptools/builtineditordocumentparser.h +++ b/src/plugins/cpptools/builtineditordocumentparser.h @@ -61,7 +61,8 @@ signals: void finished(CPlusPlus::Document::Ptr document, CPlusPlus::Snapshot snapshot); public: - static BuiltinEditorDocumentParser *get(const QString &filePath); + using Ptr = QSharedPointer<BuiltinEditorDocumentParser>; + static Ptr get(const QString &filePath); private: void updateHelper(const InMemoryInfo &info) override; diff --git a/src/plugins/cpptools/builtineditordocumentprocessor.cpp b/src/plugins/cpptools/builtineditordocumentprocessor.cpp index b61f698ab4..c6213dd668 100644 --- a/src/plugins/cpptools/builtineditordocumentprocessor.cpp +++ b/src/plugins/cpptools/builtineditordocumentprocessor.cpp @@ -125,7 +125,7 @@ BuiltinEditorDocumentProcessor::BuiltinEditorDocumentProcessor( TextEditor::TextDocument *document, bool enableSemanticHighlighter) : BaseEditorDocumentProcessor(document) - , m_parser(document->filePath().toString()) + , m_parser(new BuiltinEditorDocumentParser(document->filePath().toString())) , m_codeWarningsUpdated(false) , m_semanticHighlighter(enableSemanticHighlighter ? new CppTools::SemanticHighlighter(document) @@ -135,9 +135,9 @@ BuiltinEditorDocumentProcessor::BuiltinEditorDocumentProcessor( QSharedPointer<CppCodeModelSettings> cms = CppToolsPlugin::instance()->codeModelSettings(); - BaseEditorDocumentParser::Configuration config = m_parser.configuration(); + BaseEditorDocumentParser::Configuration config = m_parser->configuration(); config.usePrecompiledHeaders = cms->pchUsage() != CppCodeModelSettings::PchUse_None; - m_parser.setConfiguration(config); + m_parser->setConfiguration(config); if (m_semanticHighlighter) { m_semanticHighlighter->setHighlightingRunner( @@ -152,7 +152,7 @@ BuiltinEditorDocumentProcessor::BuiltinEditorDocumentProcessor( }); } - connect(&m_parser, &BuiltinEditorDocumentParser::finished, + connect(m_parser.data(), &BuiltinEditorDocumentParser::finished, this, &BuiltinEditorDocumentProcessor::onParserFinished); connect(&m_semanticInfoUpdater, &SemanticInfoUpdater::updated, this, &BuiltinEditorDocumentProcessor::onSemanticInfoUpdated); @@ -171,14 +171,14 @@ void BuiltinEditorDocumentProcessor::run() BuiltinEditorDocumentParser::InMemoryInfo(false)); } -BaseEditorDocumentParser *BuiltinEditorDocumentProcessor::parser() +BaseEditorDocumentParser::Ptr BuiltinEditorDocumentProcessor::parser() { - return &m_parser; + return m_parser; } CPlusPlus::Snapshot BuiltinEditorDocumentProcessor::snapshot() { - return m_parser.snapshot(); + return m_parser->snapshot(); } void BuiltinEditorDocumentProcessor::recalculateSemanticInfoDetached(bool force) diff --git a/src/plugins/cpptools/builtineditordocumentprocessor.h b/src/plugins/cpptools/builtineditordocumentprocessor.h index b0db31e009..8fb0fe0d6b 100644 --- a/src/plugins/cpptools/builtineditordocumentprocessor.h +++ b/src/plugins/cpptools/builtineditordocumentprocessor.h @@ -53,7 +53,7 @@ public: void recalculateSemanticInfoDetached(bool force) override; void semanticRehighlight() override; CppTools::SemanticInfo recalculateSemanticInfo() override; - BaseEditorDocumentParser *parser() override; + BaseEditorDocumentParser::Ptr parser() override; CPlusPlus::Snapshot snapshot() override; bool isParserRunning() const override; @@ -66,7 +66,7 @@ private: SemanticInfo::Source createSemanticInfoSource(bool force) const; private: - BuiltinEditorDocumentParser m_parser; + BuiltinEditorDocumentParser::Ptr m_parser; QFuture<void> m_parserFuture; CPlusPlus::Snapshot m_documentSnapshot; diff --git a/src/plugins/cpptools/cppcompletionassist.cpp b/src/plugins/cpptools/cppcompletionassist.cpp index 96f6a6efb2..bdb729b189 100644 --- a/src/plugins/cpptools/cppcompletionassist.cpp +++ b/src/plugins/cpptools/cppcompletionassist.cpp @@ -426,13 +426,13 @@ AssistInterface *InternalCompletionAssistProvider::createAssistInterface( { QTC_ASSERT(textEditorWidget, return 0); - CppModelManager *modelManager = CppModelManager::instance(); return new CppCompletionAssistInterface(filePath, textEditorWidget, + BuiltinEditorDocumentParser::get(filePath), languageFeatures, position, reason, - modelManager->workingCopy()); + CppModelManager::instance()->workingCopy()); } // ----------------- @@ -2187,11 +2187,11 @@ void CppCompletionAssistInterface::getCppSpecifics() const return; m_gotCppSpecifics = true; - if (BuiltinEditorDocumentParser *parser = BuiltinEditorDocumentParser::get(fileName())) { - parser->update(BuiltinEditorDocumentParser::InMemoryInfo(false)); - m_snapshot = parser->snapshot(); - m_headerPaths = parser->headerPaths(); - if (Document::Ptr document = parser->document()) + if (m_parser) { + m_parser->update(BuiltinEditorDocumentParser::InMemoryInfo(false)); + m_snapshot = m_parser->snapshot(); + m_headerPaths = m_parser->headerPaths(); + if (Document::Ptr document = m_parser->document()) m_languageFeatures = document->languageFeatures(); else m_languageFeatures = LanguageFeatures::defaultFeatures(); diff --git a/src/plugins/cpptools/cppcompletionassist.h b/src/plugins/cpptools/cppcompletionassist.h index 9de04d69e4..01fd44e5b8 100644 --- a/src/plugins/cpptools/cppcompletionassist.h +++ b/src/plugins/cpptools/cppcompletionassist.h @@ -31,6 +31,7 @@ #ifndef CPPCOMPLETIONASSIST_H #define CPPCOMPLETIONASSIST_H +#include "builtineditordocumentparser.h" #include "cppcompletionassistprocessor.h" #include "cppcompletionassistprovider.h" #include "cppmodelmanager.h" @@ -171,11 +172,13 @@ class CppCompletionAssistInterface : public TextEditor::AssistInterface public: CppCompletionAssistInterface(const QString &filePath, const TextEditor::TextEditorWidget *textEditorWidget, + BuiltinEditorDocumentParser::Ptr parser, const CPlusPlus::LanguageFeatures &languageFeatures, int position, TextEditor::AssistReason reason, const WorkingCopy &workingCopy) : TextEditor::AssistInterface(textEditorWidget->document(), position, filePath, reason) + , m_parser(parser) , m_gotCppSpecifics(false) , m_workingCopy(workingCopy) , m_languageFeatures(languageFeatures) @@ -204,6 +207,7 @@ public: private: void getCppSpecifics() const; + BuiltinEditorDocumentParser::Ptr m_parser; mutable bool m_gotCppSpecifics; WorkingCopy m_workingCopy; mutable CPlusPlus::Snapshot m_snapshot; diff --git a/src/plugins/cpptools/cppmodelmanager_test.cpp b/src/plugins/cpptools/cppmodelmanager_test.cpp index 7aa40b357b..1015329879 100644 --- a/src/plugins/cpptools/cppmodelmanager_test.cpp +++ b/src/plugins/cpptools/cppmodelmanager_test.cpp @@ -910,7 +910,7 @@ void CppToolsPlugin::test_modelmanager_precompiled_headers() QCOMPARE(Core::DocumentModel::openedDocuments().size(), 1); QVERIFY(mm->isCppEditor(editor)); - auto *parser = BuiltinEditorDocumentParser::get(fileName); + auto parser = BuiltinEditorDocumentParser::get(fileName); QVERIFY(parser); BaseEditorDocumentParser::Configuration config = parser->configuration(); config.usePrecompiledHeaders = true; @@ -994,7 +994,7 @@ void CppToolsPlugin::test_modelmanager_defines_per_editor() QVERIFY(mm->isCppEditor(editor)); const QString filePath = editor->document()->filePath().toString(); - BaseEditorDocumentParser *parser = BaseEditorDocumentParser::get(filePath); + const auto parser = BaseEditorDocumentParser::get(filePath); BaseEditorDocumentParser::Configuration config = parser->configuration(); config.editorDefines = editorDefines.toUtf8(); parser->setConfiguration(config); diff --git a/src/plugins/designer/gotoslot_test.cpp b/src/plugins/designer/gotoslot_test.cpp index ffd89b0d27..53ee0290ea 100644 --- a/src/plugins/designer/gotoslot_test.cpp +++ b/src/plugins/designer/gotoslot_test.cpp @@ -196,12 +196,12 @@ public: } // Compare - BuiltinEditorDocumentParser *cppDocumentParser = BuiltinEditorDocumentParser::get(cppFile); + const auto cppDocumentParser = BuiltinEditorDocumentParser::get(cppFile); QVERIFY(cppDocumentParser); const Document::Ptr cppDocument = cppDocumentParser->document(); QVERIFY(checkDiagsnosticMessages(cppDocument)); - BuiltinEditorDocumentParser *hDocumentParser = BuiltinEditorDocumentParser::get(hFile); + const auto hDocumentParser = BuiltinEditorDocumentParser::get(hFile); QVERIFY(hDocumentParser); const Document::Ptr hDocument = hDocumentParser->document(); QVERIFY(checkDiagsnosticMessages(hDocument)); diff --git a/src/plugins/texteditor/codeassist/codeassistant.cpp b/src/plugins/texteditor/codeassist/codeassistant.cpp index b7f6e40510..ffe86295c2 100644 --- a/src/plugins/texteditor/codeassist/codeassistant.cpp +++ b/src/plugins/texteditor/codeassist/codeassistant.cpp @@ -89,7 +89,6 @@ public: virtual bool eventFilter(QObject *o, QEvent *e); private: - void finalizeRequest(); void proposalComputed(); void processProposalItem(AssistProposalItem *proposalItem); void handlePrefixExpansion(const QString &newPrefix); @@ -251,7 +250,7 @@ void CodeAssistantPrivate::requestProposal(AssistReason reason, connect(m_requestRunner, &ProcessorRunner::finished, this, &CodeAssistantPrivate::proposalComputed); connect(m_requestRunner, &ProcessorRunner::finished, - this, &CodeAssistantPrivate::finalizeRequest); + m_requestRunner, &QObject::deleteLater); connect(m_requestRunner, &ProcessorRunner::finished, q, &CodeAssistant::finished); assistInterface->prepareForAsyncUse(); @@ -383,12 +382,6 @@ void CodeAssistantPrivate::handlePrefixExpansion(const QString &newPrefix) notifyChange(); } -void CodeAssistantPrivate::finalizeRequest() -{ - if (ProcessorRunner *runner = qobject_cast<ProcessorRunner *>(sender())) - delete runner; -} - void CodeAssistantPrivate::finalizeProposal() { stopAutomaticProposalTimer(); |