From c17214c53f9647116a82fef9766f14c4e0c2eda1 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Thu, 7 Jul 2022 15:46:53 +0200 Subject: [PATCH] ClangClient: Receive done() signal only once Before this change the done() signal was emitted 3 times in a row. The reason was that first one was coming synchonously from handleGotoDefinitionResult(), while 2 others were scheluded with queued connections. So, the handler for done() signal was called 3 times. The first invocation calls deleteLater() and clears the pointer, while the object is still alive. Before the request to delete later gets dispatched, the 2 remaining done() emissions are dispatched, so the handler is still called 2 times. One possible solution would be to disconnect from done() signal inside a handler. However, the done() signal shouldn't be called many times, so this fix ensures the done() is emitted only once. This fixes the following prinouts: "QCoreApplication::postEvent: Unexpected null receiver" issued twice on every follow symbol interaction. Amends 650bc260c6a40fbea034fe3c22c302ed6111d0b9 Change-Id: I9b440a80386aca3462eda323e51a76696e53fa6b Reviewed-by: Christian Kandeler --- .../clangcodemodel/clangdfollowsymbol.cpp | 31 ++++++++++++------- .../clangcodemodel/clangdfollowsymbol.h | 1 + .../clangcodemodel/clangdswitchdecldef.cpp | 26 +++++++++++----- .../clangcodemodel/clangdswitchdecldef.h | 1 + 4 files changed, 40 insertions(+), 19 deletions(-) diff --git a/src/plugins/clangcodemodel/clangdfollowsymbol.cpp b/src/plugins/clangcodemodel/clangdfollowsymbol.cpp index 4c7400914ab..102e49acf4e 100644 --- a/src/plugins/clangcodemodel/clangdfollowsymbol.cpp +++ b/src/plugins/clangcodemodel/clangdfollowsymbol.cpp @@ -135,7 +135,7 @@ public: std::set openedFiles; VirtualFunctionAssistProcessor *virtualFuncAssistProcessor = nullptr; QMetaObject::Connection focusChangedConnection; - bool finished = false; + bool done = false; }; ClangdFollowSymbol::ClangdFollowSymbol(ClangdClient *client, const QTextCursor &cursor, @@ -146,14 +146,14 @@ ClangdFollowSymbol::ClangdFollowSymbol(ClangdClient *client, const QTextCursor & openInSplit)) { // Abort if the user does something else with the document in the meantime. - connect(document, &TextDocument::contentsChanged, this, &ClangdFollowSymbol::done, + connect(document, &TextDocument::contentsChanged, this, &ClangdFollowSymbol::emitDone, Qt::QueuedConnection); if (editorWidget) { connect(editorWidget, &CppEditorWidget::cursorPositionChanged, - this, &ClangdFollowSymbol::done, Qt::QueuedConnection); + this, &ClangdFollowSymbol::emitDone, Qt::QueuedConnection); } d->focusChangedConnection = connect(qApp, &QApplication::focusChanged, - this, &ClangdFollowSymbol::done, Qt::QueuedConnection); + this, &ClangdFollowSymbol::emitDone, Qt::QueuedConnection); // Step 1: Follow the symbol via "Go to Definition". At the same time, request the // AST node corresponding to the cursor position, so we can find out whether @@ -163,7 +163,7 @@ ClangdFollowSymbol::ClangdFollowSymbol(ClangdClient *client, const QTextCursor & if (!self) return; if (!link.hasValidTarget()) { - emit self->done(); + self->emitDone(); return; } self->d->defLink = link; @@ -205,6 +205,15 @@ void ClangdFollowSymbol::clear() d->pendingGotoDefRequests.clear(); } +void ClangdFollowSymbol::emitDone() +{ + if (d->done) + return; + + d->done = true; + emit done(); +} + bool ClangdFollowSymbol::Private::defLinkIsAmbiguous() const { // Even if the call is to a virtual function, it might not be ambiguous: @@ -238,18 +247,18 @@ void ClangdFollowSymbol::Private::handleDocumentInfoResults() // If something went wrong, we just follow the original link. if (symbolsToDisplay.isEmpty()) { callback(defLink); - emit q->done(); + q->emitDone(); return; } if (symbolsToDisplay.size() == 1) { callback(symbolsToDisplay.first().second); - emit q->done(); + q->emitDone(); return; } QTC_ASSERT(virtualFuncAssistProcessor && virtualFuncAssistProcessor->running(), - emit q->done(); return); + q->emitDone(); return); virtualFuncAssistProcessor->finalize(); } @@ -301,7 +310,7 @@ void ClangdFollowSymbol::VirtualFunctionAssistProcessor::resetData(bool resetFol return; m_followSymbol->d->virtualFuncAssistProcessor = nullptr; if (resetFollowSymbolData) - emit m_followSymbol->done(); + m_followSymbol->emitDone(); m_followSymbol = nullptr; } @@ -374,7 +383,7 @@ void ClangdFollowSymbol::Private::handleGotoDefinitionResult() // No dis-ambiguation necessary. Call back with the link and finish. if (!defLinkIsAmbiguous()) { callback(defLink); - emit q->done(); + q->emitDone(); return; } @@ -408,7 +417,7 @@ void ClangdFollowSymbol::Private::handleGotoImplementationResult( // We didn't find any further candidates, so jump to the original definition link. if (allLinks.size() == 1 && pendingGotoImplRequests.isEmpty()) { callback(allLinks.first()); - emit q->done(); + q->emitDone(); return; } diff --git a/src/plugins/clangcodemodel/clangdfollowsymbol.h b/src/plugins/clangcodemodel/clangdfollowsymbol.h index 4512675d041..fffe2c42b9f 100644 --- a/src/plugins/clangcodemodel/clangdfollowsymbol.h +++ b/src/plugins/clangcodemodel/clangdfollowsymbol.h @@ -55,6 +55,7 @@ signals: void done(); private: + void emitDone(); class VirtualFunctionAssistProcessor; class VirtualFunctionAssistProvider; diff --git a/src/plugins/clangcodemodel/clangdswitchdecldef.cpp b/src/plugins/clangcodemodel/clangdswitchdecldef.cpp index d3ce7187631..28c27034f48 100644 --- a/src/plugins/clangcodemodel/clangdswitchdecldef.cpp +++ b/src/plugins/clangcodemodel/clangdswitchdecldef.cpp @@ -68,6 +68,7 @@ public: const LinkHandler callback; optional ast; optional docSymbols; + bool done = false; }; ClangdSwitchDeclDef::ClangdSwitchDeclDef(ClangdClient *client, TextDocument *doc, @@ -75,14 +76,14 @@ ClangdSwitchDeclDef::ClangdSwitchDeclDef(ClangdClient *client, TextDocument *doc : QObject(client), d(new Private(this, client, doc, cursor, editorWidget, callback)) { // Abort if the user does something else with the document in the meantime. - connect(doc, &TextDocument::contentsChanged, this, &ClangdSwitchDeclDef::done, + connect(doc, &TextDocument::contentsChanged, this, &ClangdSwitchDeclDef::emitDone, Qt::QueuedConnection); if (editorWidget) { connect(editorWidget, &CppEditorWidget::cursorPositionChanged, - this, &ClangdSwitchDeclDef::done, Qt::QueuedConnection); + this, &ClangdSwitchDeclDef::emitDone, Qt::QueuedConnection); } connect(qApp, &QApplication::focusChanged, - this, &ClangdSwitchDeclDef::done, Qt::QueuedConnection); + this, &ClangdSwitchDeclDef::emitDone, Qt::QueuedConnection); connect(client->documentSymbolCache(), &DocumentSymbolCache::gotSymbols, this, [this](const DocumentUri &uri, const DocumentSymbolsResult &symbols) { @@ -101,11 +102,11 @@ ClangdSwitchDeclDef::ClangdSwitchDeclDef(ClangdClient *client, TextDocument *doc if (!self) return; if (!d->document) { - emit done(); + emitDone(); return; } if (!ast.isValid()) { - emit done(); + emitDone(); return; } d->ast = ast; @@ -122,6 +123,15 @@ ClangdSwitchDeclDef::~ClangdSwitchDeclDef() delete d; } +void ClangdSwitchDeclDef::emitDone() +{ + if (d->done) + return; + + d->done = true; + emit done(); +} + optional ClangdSwitchDeclDef::Private::getFunctionNode() const { QTC_ASSERT(ast, return {}); @@ -160,7 +170,7 @@ QTextCursor ClangdSwitchDeclDef::Private::cursorForFunctionName(const ClangdAstN void ClangdSwitchDeclDef::Private::handleDeclDefSwitchReplies() { if (!document) { - emit q->done(); + q->emitDone(); return; } @@ -171,7 +181,7 @@ void ClangdSwitchDeclDef::Private::handleDeclDefSwitchReplies() ast->print(0); const Utils::optional functionNode = getFunctionNode(); if (!functionNode) { - emit q->done(); + q->emitDone(); return; } @@ -182,7 +192,7 @@ void ClangdSwitchDeclDef::Private::handleDeclDefSwitchReplies() client->followSymbol(document.data(), funcNameCursor, editorWidget, callback, true, false); } - emit q->done(); + q->emitDone(); } } // namespace ClangCodeModel::Internal diff --git a/src/plugins/clangcodemodel/clangdswitchdecldef.h b/src/plugins/clangcodemodel/clangdswitchdecldef.h index 8c278bf6555..44e296686db 100644 --- a/src/plugins/clangcodemodel/clangdswitchdecldef.h +++ b/src/plugins/clangcodemodel/clangdswitchdecldef.h @@ -52,6 +52,7 @@ signals: void done(); private: + void emitDone(); class Private; Private * const d; };