sammccall added inline comments.
================ Comment at: clang-tools-extra/clangd/ClangdServer.cpp:180 + for (auto &Mod : *Modules) + Mod.blockUntilIdle(Deadline::infinity()); + } ---------------- kadircet wrote: > why is our contract saying that just calling `stop` is not enough? > i think clangdserver should just signal shutdown to modules, and our contract > should say that server facilities will be undefined from this point forward. > that way modules accessing the facilities, could block stop until they are > done, and never make use of it afterwards? it'll make modules a little more > complicated, at the very least they would need some stricter control whenever > they are accessing facilities, but I think it is worth for keeping > clangdserver shutdown cleaner. wdyt? We need Module::blockUntilIdle anyway for tests and stuff. And in practice requesting shutdown of background work doesn't naturally block until that work is complete IME. So we could ask every module to call blockUntilIdle() in stop(), or make stop() non-virtual and have it wrap requestStop()+blockUntilIdle(). Neither really seems simpler/cleaner to me overall, and it means modules shut down in serial instead of parallel. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96755/new/ https://reviews.llvm.org/D96755 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
