[clangd] Fix threading bugs in (not-yet-used) BackgroundIndex, re-enable test.

[sammccall]

Summary

One relatively boring bug: forgot to notify the CV after enqueue.

One much more fun bug: the thread member could access instance variables before
they were initialized. Although the thread was last in the init list, QueueCV
etc were listed after Thread in the class, so their default constructors raced
with the thread itself.
We have to get very unlucky to lose this race, I saw it 0.02% of the time.

clang-tools-extra/trunk/clangd/index/Background.h

private:
   FileSymbols IndexedSymbols; // Index contents.
   using Hash = decltype(llvm::SHA1::hash({}));
   llvm::StringMap<Hash> FileHash; // Digest of indexed file.
 
   // queue management
   using Task = std::function<void()>; // FIXME: use multiple worker threads.
   void run(); // Main loop executed by Thread. Runs tasks from Queue.
   void enqueueLocked(tooling::CompileCommand Cmd);
-  std::thread Thread;
   std::mutex QueueMu;
   unsigned NumActiveTasks = 0; // Only idle when queue is empty *and* no tasks.
   std::condition_variable QueueCV;
   bool ShouldStop = false;
   std::deque<Task> Queue;
+  std::thread Thread; // Must be last, spawned thread reads instance vars.
 };
 
 } // namespace clangd
 } // namespace clang
 
 #endif

clang-tools-extra/trunk/clangd/index/Background.cpp

 
 void BackgroundIndex::blockUntilIdleForTest() {
   std::unique_lock<std::mutex> Lock(QueueMu);
   QueueCV.wait(Lock, [&] { return Queue.empty() && NumActiveTasks == 0; });
 }
 
 void BackgroundIndex::enqueue(StringRef Directory,
                               tooling::CompileCommand Cmd) {
+  {
     std::lock_guard<std::mutex> Lock(QueueMu);
     enqueueLocked(std::move(Cmd));
   }
+  QueueCV.notify_all();
+}
 
 void BackgroundIndex::enqueueAll(StringRef Directory,
                                  const tooling::CompilationDatabase &CDB) {
   trace::Span Tracer("BackgroundIndexEnqueueCDB");
   // FIXME: this function may be slow. Perhaps enqueue a task to re-read the CDB
   // from disk and enqueue the commands asynchronously?
   auto Cmds = CDB.getAllCompileCommands();
   SPAN_ATTACH(Tracer, "commands", int64_t(Cmds.size()));

clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp

 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "index/Background.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 using testing::UnorderedElementsAre;
 
 namespace clang {
 namespace clangd {
 
 MATCHER_P(Named, N, "") { return arg.Name == N; }
 
-// Temporarily disabled: test timing out on buildbots.
-#if 0
 TEST(BackgroundIndexTest, IndexTwoFiles) {
   MockFSProvider FS;
   // a.h yields different symbols when included by A.cc vs B.cc.
   // Currently we store symbols for each TU, so we get both.
   FS.Files[testPath("root/A.h")] = "void a_h(); void NAME(){}";
   FS.Files[testPath("root/A.cc")] = "#include \"A.h\"";
   FS.Files[testPath("root/B.cc")] = "#define NAME bar\n#include \"A.h\"";
   BackgroundIndex Idx(Context::empty(), "", FS);
 
   tooling::CompileCommand Cmd;
   Cmd.Filename = testPath("root/A.cc");
   Cmd.Directory = testPath("root");
   Cmd.CommandLine = {"clang++", "-DNAME=foo", testPath("root/A.cc")};
   Idx.enqueue(testPath("root"), Cmd);
   Cmd.CommandLine.back() = Cmd.Filename = testPath("root/B.cc");
   Idx.enqueue(testPath("root"), Cmd);
 
   Idx.blockUntilIdleForTest();
   EXPECT_THAT(runFuzzyFind(Idx, ""),
               UnorderedElementsAre(Named("a_h"), Named("foo"), Named("bar")));
 }
-#endif
 
 } // namespace clangd
 } // namespace clang