ClangCodeModel: fix crash on followSymbol

Big files are loaded in chunks inside a QEventLoop. If follow symbol is
triggered via mouse and opens such a big file and the mouse is moved
while loading that file ClangdFollowSymbol got deleted while opening the
file in ClangdClient::followSymbol. Instead of the hard deletion just
cancel that follow symbol operation and make sure done is emitted
afterwards. The handling of that done signal takes care of the deletion
of that follow symbol operation.

Change-Id: Iba4ad6abb541186c2f26506f82fe1bc582818fca
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/plugins/clangcodemodel/clangdclient.cpp

@@ -977,6 +977,12 @@ MessageId ClangdClient::requestSymbolInfo(const Utils::FilePath &filePath, const
     return symReq.id();
 }
 
+#ifdef WITH_TESTS
+ClangdFollowSymbol *ClangdClient::currentFollowSymbolOperation()
+{
+    return d->followSymbol;
+}
+#endif
 
 void ClangdClient::followSymbol(TextDocument *document,
         const QTextCursor &cursor,
@@ -989,8 +995,8 @@ void ClangdClient::followSymbol(TextDocument *document,
 {
     QTC_ASSERT(documentOpen(document), openDocument(document));
 
-    delete d->followSymbol;
-    d->followSymbol = nullptr;
+    if (d->followSymbol)
+        d->followSymbol->cancel();
 
     const QTextCursor adjustedCursor = d->adjustedCursor(cursor, document);
     if (followTo == FollowTo::SymbolDef && !resolveTarget) {
@@ -1000,12 +1006,14 @@ void ClangdClient::followSymbol(TextDocument *document,
 
     qCDebug(clangdLog) << "follow symbol requested" << document->filePath()
                        << adjustedCursor.blockNumber() << adjustedCursor.positionInBlock();
-    d->followSymbol = new ClangdFollowSymbol(this, adjustedCursor, editorWidget, document, callback,
-                                             followTo, openInSplit);
-    connect(d->followSymbol, &ClangdFollowSymbol::done, this, [this] {
-        d->followSymbol->deleteLater();
-        d->followSymbol = nullptr;
+    auto clangdFollowSymbol = new ClangdFollowSymbol(this, adjustedCursor, editorWidget, document,
+                                                     callback, followTo, openInSplit);
+    connect(clangdFollowSymbol, &ClangdFollowSymbol::done, this, [this, clangdFollowSymbol] {
+        clangdFollowSymbol->deleteLater();
+        if (clangdFollowSymbol == d->followSymbol)
+            d->followSymbol = nullptr;
     });
+    d->followSymbol = clangdFollowSymbol;
 }
 
 void ClangdClient::switchDeclDef(TextDocument *document, const QTextCursor &cursor,

src/plugins/clangcodemodel/clangdclient.h

@@ -37,6 +37,8 @@ void setupClangdConfigFile();
 
 enum class FollowTo { SymbolDef, SymbolType };
 
+class ClangdFollowSymbol;
+
 class ClangdClient : public LanguageClient::Client
 {
     Q_OBJECT
@@ -117,6 +119,10 @@ public:
             const LanguageServerProtocol::Position &position,
             const SymbolInfoHandler &handler);
 
+#ifdef WITH_TESTS
+    ClangdFollowSymbol *currentFollowSymbolOperation();
+#endif
+
 signals:
     void indexingFinished();
     void foundReferences(const Utils::SearchResultItems &items);

src/plugins/clangcodemodel/clangdfollowsymbol.cpp

@@ -90,6 +90,7 @@ public:
     void closeTempDocuments();
     bool addOpenFile(const FilePath &filePath);
     bool defLinkIsAmbiguous() const;
+    void cancel();
 
     ClangdFollowSymbol * const q;
     ClangdClient * const client;
@@ -169,18 +170,17 @@ ClangdFollowSymbol::ClangdFollowSymbol(ClangdClient *client, const QTextCursor &
 
 ClangdFollowSymbol::~ClangdFollowSymbol()
 {
-    d->closeTempDocuments();
-    if (d->virtualFuncAssistProcessor)
-        d->virtualFuncAssistProcessor->resetData(false);
-    for (const MessageId &id : std::as_const(d->pendingSymbolInfoRequests))
-        d->client->cancelRequest(id);
-    for (const MessageId &id : std::as_const(d->pendingGotoImplRequests))
-        d->client->cancelRequest(id);
-    for (const MessageId &id : std::as_const(d->pendingGotoDefRequests))
-        d->client->cancelRequest(id);
+    d->cancel();
     delete d;
 }
 
+void ClangdFollowSymbol::cancel()
+{
+    d->cancel();
+    clear();
+    emitDone();
+}
+
 void ClangdFollowSymbol::clear()
 {
     d->openedFiles.clear();
@@ -221,6 +221,19 @@ bool ClangdFollowSymbol::Private::defLinkIsAmbiguous() const
     return true;
 }
 
+void ClangdFollowSymbol::Private::cancel()
+{
+    closeTempDocuments();
+    if (virtualFuncAssistProcessor)
+        virtualFuncAssistProcessor->resetData(false);
+    for (const MessageId &id : std::as_const(pendingSymbolInfoRequests))
+        client->cancelRequest(id);
+    for (const MessageId &id : std::as_const(pendingGotoImplRequests))
+        client->cancelRequest(id);
+    for (const MessageId &id : std::as_const(pendingGotoDefRequests))
+        client->cancelRequest(id);
+}
+
 bool ClangdFollowSymbol::Private::addOpenFile(const FilePath &filePath)
 {
     return openedFiles.insert(filePath).second;

src/plugins/clangcodemodel/clangdfollowsymbol.h

@@ -28,6 +28,7 @@ public:
                        TextEditor::TextDocument *document, const Utils::LinkHandler &callback,
                        FollowTo followTo, bool openInSplit);
     ~ClangdFollowSymbol();
+    void cancel();
     void clear();
 
 signals:

src/plugins/clangcodemodel/test/clangdtests.cpp

@@ -4,6 +4,7 @@
 #include "clangdtests.h"
 
 #include "../clangdclient.h"
+#include "../clangdfollowsymbol.h"
 #include "../clangmodelmanagersupport.h"
 
 #include <coreplugin/editormanager/editormanager.h>
@@ -428,6 +429,37 @@ void ClangdTestFollowSymbol::test()
     QCOMPARE(actualLink.targetColumn + 1, targetColumn);
 }
 
+// Make sure it is safe to call follow symbol in a follow symbol handler. Since follow symbol
+// potentially opens a file that gets loaded in chunks which handles user events inbetween
+// the chunks we can potentially call a follow symbol while currently handling another one.
+void ClangdTestFollowSymbol::testFollowSymbolInHandler()
+{
+    TextEditor::TextDocument *const doc = document("header.h");
+    QVERIFY(doc);
+    QTimer timer;
+    timer.setSingleShot(true);
+    QEventLoop loop;
+    QTextCursor cursor(doc->document());
+    const int pos = Text::positionInText(doc->document(), 48, 9);
+    cursor.setPosition(pos);
+    QObject::connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit);
+
+    bool deleted = false;
+
+    const auto handler = [&](const Link &) {
+        client()->followSymbol(doc, cursor, nullptr, [&](const Link &) { loop.quit(); }, true,
+                               FollowTo::SymbolDef, false);
+        QVERIFY(!deleted);
+    };
+
+    client()->followSymbol(doc, cursor, nullptr, handler, true, FollowTo::SymbolDef, false);
+    QVERIFY(client()->currentFollowSymbolOperation());
+    connect(client()->currentFollowSymbolOperation(), &QObject::destroyed, [&] { deleted = true; });
+    timer.start(10000);
+    loop.exec();
+    QVERIFY(timer.isActive());
+    timer.stop();
+}
 
 ClangdTestLocalReferences::ClangdTestLocalReferences()
 {

src/plugins/clangcodemodel/test/clangdtests.h

@@ -86,6 +86,7 @@ public:
 private slots:
     void test_data();
     void test();
+    void testFollowSymbolInHandler();
 };
 
 class ClangdTestLocalReferences : public ClangdTest