From f773c09f33a81880ea38199b5fe49d6ba67b8b69 Mon Sep 17 00:00:00 2001 From: David Schulz Date: Wed, 5 Apr 2023 12:48:19 +0200 Subject: [PATCH] TextEditor: fix crash on reload Since TextDocument::openImpl potentially processes events it could delete TextMarks. So tracking them in TextDocument::reload can be considered unsafe. Track them in TextDocumentLayout instead and remove the tracked mark if it gets deleted while reloading the document. Task-number: QTCREATORBUG-29004 Change-Id: I9d0478e9c763b49f145c1bbaeed1a0b602757014 Reviewed-by: Jarek Kobus --- src/plugins/texteditor/textdocument.cpp | 5 ++--- src/plugins/texteditor/textdocumentlayout.cpp | 21 +++++++++++++++++-- src/plugins/texteditor/textdocumentlayout.h | 5 ++++- src/plugins/texteditor/textmark.cpp | 2 ++ src/plugins/texteditor/textmark.h | 6 ++++++ 5 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/plugins/texteditor/textdocument.cpp b/src/plugins/texteditor/textdocument.cpp index 4a7e80f52ce..6276d731d06 100644 --- a/src/plugins/texteditor/textdocument.cpp +++ b/src/plugins/texteditor/textdocument.cpp @@ -836,15 +836,14 @@ bool TextDocument::reload(QString *errorString, const FilePath &realFilePath) emit aboutToReload(); auto documentLayout = qobject_cast(d->m_document.documentLayout()); - TextMarks marks; if (documentLayout) - marks = documentLayout->documentClosing(); // removes text marks non-permanently + documentLayout->documentAboutToReload(); // removes text marks non-permanently bool success = openImpl(errorString, filePath(), realFilePath, /*reload =*/true) == OpenResult::Success; if (documentLayout) - documentLayout->documentReloaded(marks, this); // re-adds text marks + documentLayout->documentReloaded(this); // re-adds text marks emit reloadFinished(success); return success; } diff --git a/src/plugins/texteditor/textdocumentlayout.cpp b/src/plugins/texteditor/textdocumentlayout.cpp index 2b6b77c374e..c38c75bd5e0 100644 --- a/src/plugins/texteditor/textdocumentlayout.cpp +++ b/src/plugins/texteditor/textdocumentlayout.cpp @@ -656,6 +656,7 @@ QSizeF TextDocumentLayout::documentSize() const TextMarks TextDocumentLayout::documentClosing() { + QTC_ASSERT(m_reloadMarks.isEmpty(), resetReloadMarks()); TextMarks marks; for (QTextBlock block = document()->begin(); block.isValid(); block = block.next()) { if (auto data = static_cast(block.userData())) @@ -664,9 +665,17 @@ TextMarks TextDocumentLayout::documentClosing() return marks; } -void TextDocumentLayout::documentReloaded(TextMarks marks, TextDocument *baseTextDocument) +void TextDocumentLayout::documentAboutToReload() { - for (TextMark *mark : std::as_const(marks)) { + m_reloadMarks = documentClosing(); + for (TextMark *mark : std::as_const(m_reloadMarks)) + mark->setDeleteCallback([this, mark] { m_reloadMarks.removeOne(mark); }); +} + +void TextDocumentLayout::documentReloaded(TextDocument *baseTextDocument) +{ + for (TextMark *mark : std::as_const(m_reloadMarks)) { + mark->setDeleteCallback({}); int blockNumber = mark->lineNumber() - 1; QTextBlock block = document()->findBlockByNumber(blockNumber); if (block.isValid()) { @@ -680,6 +689,7 @@ void TextDocumentLayout::documentReloaded(TextMarks marks, TextDocument *baseTex mark->removedFromEditor(); } } + m_reloadMarks.clear(); requestUpdate(); } @@ -721,6 +731,13 @@ void TextDocumentLayout::requestUpdateNow() requestUpdate(); } +void TextDocumentLayout::resetReloadMarks() +{ + for (TextMark *mark : std::as_const(m_reloadMarks)) + mark->setDeleteCallback({}); + m_reloadMarks.clear(); +} + static QRectF replacementBoundingRect(const QTextDocument *replacement) { QTC_ASSERT(replacement, return {}); diff --git a/src/plugins/texteditor/textdocumentlayout.h b/src/plugins/texteditor/textdocumentlayout.h index 19278ad93c9..d6f847e4810 100644 --- a/src/plugins/texteditor/textdocumentlayout.h +++ b/src/plugins/texteditor/textdocumentlayout.h @@ -245,7 +245,8 @@ public: QRectF blockBoundingRect(const QTextBlock &block) const override; TextMarks documentClosing(); - void documentReloaded(TextMarks marks, TextDocument *baseextDocument); + void documentAboutToReload(); + void documentReloaded(TextDocument *baseextDocument); void updateMarksLineNumber(); void updateMarksBlock(const QTextBlock &block); void scheduleUpdate(); @@ -253,6 +254,8 @@ public: private: bool m_updateScheduled = false; + TextMarks m_reloadMarks; + void resetReloadMarks(); signals: void updateExtraArea(); diff --git a/src/plugins/texteditor/textmark.cpp b/src/plugins/texteditor/textmark.cpp index b7ded20d3fc..0609121bffc 100644 --- a/src/plugins/texteditor/textmark.cpp +++ b/src/plugins/texteditor/textmark.cpp @@ -81,6 +81,8 @@ TextMark::~TextMark() TextMarkRegistry::remove(this); if (m_baseTextDocument) m_baseTextDocument->removeMark(this); + if (m_deleteCallback) + m_deleteCallback(); m_baseTextDocument = nullptr; } diff --git a/src/plugins/texteditor/textmark.h b/src/plugins/texteditor/textmark.h index 72712557058..927255161d5 100644 --- a/src/plugins/texteditor/textmark.h +++ b/src/plugins/texteditor/textmark.h @@ -118,12 +118,15 @@ public: bool isLocationMarker() const;; void setIsLocationMarker(bool newIsLocationMarker); + protected: void setSettingsPage(Utils::Id settingsPage); private: Q_DISABLE_COPY(TextMark) + void setDeleteCallback(const std::function &callback) { m_deleteCallback = callback; }; + TextDocument *m_baseTextDocument = nullptr; Utils::FilePath m_fileName; int m_lineNumber = 0; @@ -141,6 +144,9 @@ private: QVector m_actions; // FIXME Remove in master std::function()> m_actionsProvider; Utils::Id m_settingsPage; + std::function m_deleteCallback; + + friend class TextDocumentLayout; }; } // namespace TextEditor