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 <jaroslaw.kobus@qt.io>

src/plugins/texteditor/textdocument.cpp

@@ -836,15 +836,14 @@ bool TextDocument::reload(QString *errorString, const FilePath &realFilePath)
     emit aboutToReload();
     auto documentLayout =
         qobject_cast<TextDocumentLayout*>(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;
 }

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<TextBlockUserData *>(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 {});

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();

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;
 }
 

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<void()> &callback) { m_deleteCallback = callback; };
+
     TextDocument *m_baseTextDocument = nullptr;
     Utils::FilePath m_fileName;
     int m_lineNumber = 0;
@@ -141,6 +144,9 @@ private:
     QVector<QAction *> m_actions; // FIXME Remove in master
     std::function<QList<QAction *>()> m_actionsProvider;
     Utils::Id m_settingsPage;
+    std::function<void()> m_deleteCallback;
+
+    friend class TextDocumentLayout;
 };
 
 } // namespace TextEditor