Skip to content

Commit 05323da

Browse files
kadircetjph-13
authored andcommitted
[clangd] Explicitly block until async task completes (llvm#130077)
We started seeing some use-after-frees starting with llvm#125433. This patch ensures we explicitly block for the async task, if there's one, before destructing `std::future`, independent of the stdlib implementation.
1 parent d84797f commit 05323da

File tree

2 files changed

+11
-6
lines changed

2 files changed

+11
-6
lines changed

clang-tools-extra/clangd/ClangdServer.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -460,18 +460,24 @@ void ClangdServer::codeComplete(PathRef File, Position Pos,
460460
CodeCompleteResult Result = clangd::codeComplete(
461461
File, Pos, IP->Preamble, ParseInput, CodeCompleteOpts,
462462
SpecFuzzyFind ? &*SpecFuzzyFind : nullptr);
463+
// We don't want `codeComplete` to wait for the async call if it doesn't use
464+
// the result (e.g. non-index completion, speculation fails), so that `CB`
465+
// is called as soon as results are available.
463466
{
464467
clang::clangd::trace::Span Tracer("Completion results callback");
465468
CB(std::move(Result));
466469
}
467-
if (SpecFuzzyFind && SpecFuzzyFind->NewReq) {
470+
if (!SpecFuzzyFind)
471+
return;
472+
if (SpecFuzzyFind->NewReq) {
468473
std::lock_guard<std::mutex> Lock(CachedCompletionFuzzyFindRequestMutex);
469474
CachedCompletionFuzzyFindRequestByFile[File] = *SpecFuzzyFind->NewReq;
470475
}
471-
// SpecFuzzyFind is only destroyed after speculative fuzzy find finishes.
472-
// We don't want `codeComplete` to wait for the async call if it doesn't use
473-
// the result (e.g. non-index completion, speculation fails), so that `CB`
474-
// is called as soon as results are available.
476+
// Explicitly block until async task completes, this is fine as we've
477+
// already provided reply to the client and running as a preamble task
478+
// (i.e. won't block other preamble tasks).
479+
if (SpecFuzzyFind->Result.valid())
480+
SpecFuzzyFind->Result.wait();
475481
};
476482

477483
// We use a potentially-stale preamble because latency is critical here.

clang-tools-extra/clangd/CodeComplete.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,6 @@ struct SpeculativeFuzzyFind {
274274
/// Set by `codeComplete()`. This can be used by callers to update cache.
275275
std::optional<FuzzyFindRequest> NewReq;
276276
/// The result is consumed by `codeComplete()` if speculation succeeded.
277-
/// NOTE: the destructor will wait for the async call to finish.
278277
std::future<std::pair<bool /*Incomplete*/, SymbolSlab>> Result;
279278
};
280279

0 commit comments

Comments
 (0)