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 650bc260c6

Change-Id: I9b440a80386aca3462eda323e51a76696e53fa6b
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>
This commit is contained in:
Jarek Kobus
2022-07-07 15:46:53 +02:00
parent 6a6ea17a16
commit c17214c53f
4 changed files with 40 additions and 19 deletions

View File

@@ -135,7 +135,7 @@ public:
std::set<FilePath> openedFiles; std::set<FilePath> openedFiles;
VirtualFunctionAssistProcessor *virtualFuncAssistProcessor = nullptr; VirtualFunctionAssistProcessor *virtualFuncAssistProcessor = nullptr;
QMetaObject::Connection focusChangedConnection; QMetaObject::Connection focusChangedConnection;
bool finished = false; bool done = false;
}; };
ClangdFollowSymbol::ClangdFollowSymbol(ClangdClient *client, const QTextCursor &cursor, ClangdFollowSymbol::ClangdFollowSymbol(ClangdClient *client, const QTextCursor &cursor,
@@ -146,14 +146,14 @@ ClangdFollowSymbol::ClangdFollowSymbol(ClangdClient *client, const QTextCursor &
openInSplit)) openInSplit))
{ {
// Abort if the user does something else with the document in the meantime. // 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); Qt::QueuedConnection);
if (editorWidget) { if (editorWidget) {
connect(editorWidget, &CppEditorWidget::cursorPositionChanged, connect(editorWidget, &CppEditorWidget::cursorPositionChanged,
this, &ClangdFollowSymbol::done, Qt::QueuedConnection); this, &ClangdFollowSymbol::emitDone, Qt::QueuedConnection);
} }
d->focusChangedConnection = connect(qApp, &QApplication::focusChanged, 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 // 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 // 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) if (!self)
return; return;
if (!link.hasValidTarget()) { if (!link.hasValidTarget()) {
emit self->done(); self->emitDone();
return; return;
} }
self->d->defLink = link; self->d->defLink = link;
@@ -205,6 +205,15 @@ void ClangdFollowSymbol::clear()
d->pendingGotoDefRequests.clear(); d->pendingGotoDefRequests.clear();
} }
void ClangdFollowSymbol::emitDone()
{
if (d->done)
return;
d->done = true;
emit done();
}
bool ClangdFollowSymbol::Private::defLinkIsAmbiguous() const bool ClangdFollowSymbol::Private::defLinkIsAmbiguous() const
{ {
// Even if the call is to a virtual function, it might not be ambiguous: // 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 something went wrong, we just follow the original link.
if (symbolsToDisplay.isEmpty()) { if (symbolsToDisplay.isEmpty()) {
callback(defLink); callback(defLink);
emit q->done(); q->emitDone();
return; return;
} }
if (symbolsToDisplay.size() == 1) { if (symbolsToDisplay.size() == 1) {
callback(symbolsToDisplay.first().second); callback(symbolsToDisplay.first().second);
emit q->done(); q->emitDone();
return; return;
} }
QTC_ASSERT(virtualFuncAssistProcessor && virtualFuncAssistProcessor->running(), QTC_ASSERT(virtualFuncAssistProcessor && virtualFuncAssistProcessor->running(),
emit q->done(); return); q->emitDone(); return);
virtualFuncAssistProcessor->finalize(); virtualFuncAssistProcessor->finalize();
} }
@@ -301,7 +310,7 @@ void ClangdFollowSymbol::VirtualFunctionAssistProcessor::resetData(bool resetFol
return; return;
m_followSymbol->d->virtualFuncAssistProcessor = nullptr; m_followSymbol->d->virtualFuncAssistProcessor = nullptr;
if (resetFollowSymbolData) if (resetFollowSymbolData)
emit m_followSymbol->done(); m_followSymbol->emitDone();
m_followSymbol = nullptr; m_followSymbol = nullptr;
} }
@@ -374,7 +383,7 @@ void ClangdFollowSymbol::Private::handleGotoDefinitionResult()
// No dis-ambiguation necessary. Call back with the link and finish. // No dis-ambiguation necessary. Call back with the link and finish.
if (!defLinkIsAmbiguous()) { if (!defLinkIsAmbiguous()) {
callback(defLink); callback(defLink);
emit q->done(); q->emitDone();
return; return;
} }
@@ -408,7 +417,7 @@ void ClangdFollowSymbol::Private::handleGotoImplementationResult(
// We didn't find any further candidates, so jump to the original definition link. // We didn't find any further candidates, so jump to the original definition link.
if (allLinks.size() == 1 && pendingGotoImplRequests.isEmpty()) { if (allLinks.size() == 1 && pendingGotoImplRequests.isEmpty()) {
callback(allLinks.first()); callback(allLinks.first());
emit q->done(); q->emitDone();
return; return;
} }

View File

@@ -55,6 +55,7 @@ signals:
void done(); void done();
private: private:
void emitDone();
class VirtualFunctionAssistProcessor; class VirtualFunctionAssistProcessor;
class VirtualFunctionAssistProvider; class VirtualFunctionAssistProvider;

View File

@@ -68,6 +68,7 @@ public:
const LinkHandler callback; const LinkHandler callback;
optional<ClangdAstNode> ast; optional<ClangdAstNode> ast;
optional<DocumentSymbolsResult> docSymbols; optional<DocumentSymbolsResult> docSymbols;
bool done = false;
}; };
ClangdSwitchDeclDef::ClangdSwitchDeclDef(ClangdClient *client, TextDocument *doc, 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)) : QObject(client), d(new Private(this, client, doc, cursor, editorWidget, callback))
{ {
// Abort if the user does something else with the document in the meantime. // 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); Qt::QueuedConnection);
if (editorWidget) { if (editorWidget) {
connect(editorWidget, &CppEditorWidget::cursorPositionChanged, connect(editorWidget, &CppEditorWidget::cursorPositionChanged,
this, &ClangdSwitchDeclDef::done, Qt::QueuedConnection); this, &ClangdSwitchDeclDef::emitDone, Qt::QueuedConnection);
} }
connect(qApp, &QApplication::focusChanged, connect(qApp, &QApplication::focusChanged,
this, &ClangdSwitchDeclDef::done, Qt::QueuedConnection); this, &ClangdSwitchDeclDef::emitDone, Qt::QueuedConnection);
connect(client->documentSymbolCache(), &DocumentSymbolCache::gotSymbols, this, connect(client->documentSymbolCache(), &DocumentSymbolCache::gotSymbols, this,
[this](const DocumentUri &uri, const DocumentSymbolsResult &symbols) { [this](const DocumentUri &uri, const DocumentSymbolsResult &symbols) {
@@ -101,11 +102,11 @@ ClangdSwitchDeclDef::ClangdSwitchDeclDef(ClangdClient *client, TextDocument *doc
if (!self) if (!self)
return; return;
if (!d->document) { if (!d->document) {
emit done(); emitDone();
return; return;
} }
if (!ast.isValid()) { if (!ast.isValid()) {
emit done(); emitDone();
return; return;
} }
d->ast = ast; d->ast = ast;
@@ -122,6 +123,15 @@ ClangdSwitchDeclDef::~ClangdSwitchDeclDef()
delete d; delete d;
} }
void ClangdSwitchDeclDef::emitDone()
{
if (d->done)
return;
d->done = true;
emit done();
}
optional<ClangdAstNode> ClangdSwitchDeclDef::Private::getFunctionNode() const optional<ClangdAstNode> ClangdSwitchDeclDef::Private::getFunctionNode() const
{ {
QTC_ASSERT(ast, return {}); QTC_ASSERT(ast, return {});
@@ -160,7 +170,7 @@ QTextCursor ClangdSwitchDeclDef::Private::cursorForFunctionName(const ClangdAstN
void ClangdSwitchDeclDef::Private::handleDeclDefSwitchReplies() void ClangdSwitchDeclDef::Private::handleDeclDefSwitchReplies()
{ {
if (!document) { if (!document) {
emit q->done(); q->emitDone();
return; return;
} }
@@ -171,7 +181,7 @@ void ClangdSwitchDeclDef::Private::handleDeclDefSwitchReplies()
ast->print(0); ast->print(0);
const Utils::optional<ClangdAstNode> functionNode = getFunctionNode(); const Utils::optional<ClangdAstNode> functionNode = getFunctionNode();
if (!functionNode) { if (!functionNode) {
emit q->done(); q->emitDone();
return; return;
} }
@@ -182,7 +192,7 @@ void ClangdSwitchDeclDef::Private::handleDeclDefSwitchReplies()
client->followSymbol(document.data(), funcNameCursor, editorWidget, callback, client->followSymbol(document.data(), funcNameCursor, editorWidget, callback,
true, false); true, false);
} }
emit q->done(); q->emitDone();
} }
} // namespace ClangCodeModel::Internal } // namespace ClangCodeModel::Internal

View File

@@ -52,6 +52,7 @@ signals:
void done(); void done();
private: private:
void emitDone();
class Private; class Private;
Private * const d; Private * const d;
}; };