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

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL344595: [clangd] Fix threading bugs in (not-yet-used) BackgroundIndex, re-enable test. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D53313

Files:
  clang-tools-extra/trunk/clangd/index/Background.cpp
  clang-tools-extra/trunk/clangd/index/Background.h
  clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
@@ -11,8 +11,6 @@
 
 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.
@@ -34,7 +32,6 @@
   EXPECT_THAT(runFuzzyFind(Idx, ""),
               UnorderedElementsAre(Named("a_h"), Named("foo"), Named("bar")));
 }
-#endif
 
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/index/Background.h
===================================================================
--- clang-tools-extra/trunk/clangd/index/Background.h
+++ clang-tools-extra/trunk/clangd/index/Background.h
@@ -65,12 +65,12 @@
   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
Index: clang-tools-extra/trunk/clangd/index/Background.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/index/Background.cpp
+++ clang-tools-extra/trunk/clangd/index/Background.cpp
@@ -75,8 +75,11 @@
 
 void BackgroundIndex::enqueue(StringRef Directory,
                               tooling::CompileCommand Cmd) {
-  std::lock_guard<std::mutex> Lock(QueueMu);
-  enqueueLocked(std::move(Cmd));
+  {
+    std::lock_guard<std::mutex> Lock(QueueMu);
+    enqueueLocked(std::move(Cmd));
+  }
+  QueueCV.notify_all();
 }
 
 void BackgroundIndex::enqueueAll(StringRef Directory,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53313.169798.patch
Type: text/x-patch
Size: 2153 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181016/31b732d1/attachment.bin>