bnbarham added inline comments.
================ Comment at: clang-tools-extra/clangd/ClangdServer.cpp:1008-1010 + // TUScheduler is the only thing that starts background indexing work. + if (IndexTasks && !IndexTasks->wait(timeoutSeconds(TimeoutSeconds))) + return false; ---------------- @sammccall shouldn't we also be waiting for this to finish when `ClangdServer` is destroyed? IIUC right now the both `FileIndex` itself (stored in `ClangdServer`) and the actual `UpdateIndexCallbacks` (stored in `TUScheduler`) can be freed while `indexStdlib` is running asynchronously, resulting in a use-after-free on eg. `FIndex->updatePreamble(std::move(IF))`. I was confused as to why this wasn't happening in the tests, but these lines would explain it 😅 Adding a `IndexTasks->wait()` to `~ClangdServer` fixes the crash I'm seeing in the sourcekit-lsp tests (see https://github.com/apple/llvm-project/pull/5837), though ideally we (sourcekit-lsp) wouldn't be running any indexing at all. As far as I can tell there's no way to turn off dynamic indexing now though, except for `StandardLibrary` indexing through the config file (but not from clangd args)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115232/new/ https://reviews.llvm.org/D115232 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
