[PATCH] D96856: [clangd] Narrow and document a loophole in blockUntilIdle

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Doh, sorry about the garbled patch



================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:893
+  for (llvm::Optional<double> Timeout :
+       {TimeoutSeconds, TimeoutSeconds, llvm::Optional<double>(0)}) {
+    if (!CDB.blockUntilIdle(timeoutSeconds(Timeout)))
----------------
kadircet wrote:
> this is extending the Deadline in theory, e.g. if user requested idleness in 10 seconds, this can now wait for up to 20 seconds. but this was possible in the previous case too, e.g. CDB could block for 10 seconds, and then bgindex would block for another 10 seconds, and mentioned this is only for tests, so should be fine (but might be worth mentioning in the comments.)
Right - the idea with Deadline is it's something absolute you can pass around, and here we are creating a new one for each call.

This is wrong, but it's consistent with the code before this patch and I'd rather not fix both at once. Added a FIXME to the header.

(To fix this we'd want to change the signature of BackgroundIndex to take a Deadline too)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96856/new/

https://reviews.llvm.org/D96856