[PATCH] D72090: [Support] Fix races during thread pool teardown

[David Zarzycki via Phabricator via llvm-commits]

davezarzycki updated this revision to Diff 236025.
davezarzycki retitled this revision from "[Support] Fix race during thread pool teardown" to "[Support] Fix races during thread pool teardown".
davezarzycki edited the summary of this revision.
davezarzycki added a comment.
Herald added a subscriber: jfb.

Rather than a targeted fix, let's just clean up the code and fix other bugs along the way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72090

Files:
  lib/Support/Parallel.cpp


Index: lib/Support/Parallel.cpp
===================================================================
--- lib/Support/Parallel.cpp
+++ lib/Support/Parallel.cpp
@@ -11,11 +11,10 @@
 
 #if LLVM_ENABLE_THREADS
 
-#include "llvm/Support/Threading.h"
-
 #include <atomic>
 #include <stack>
 #include <thread>
+#include <vector>
 
 namespace llvm {
 namespace parallel {
@@ -36,16 +35,12 @@
 ///   in filo order.
 class ThreadPoolExecutor : public Executor {
 public:
-  explicit ThreadPoolExecutor(unsigned ThreadCount = hardware_concurrency())
-      : Done(ThreadCount) {
-    // Spawn all but one of the threads in another thread as spawning threads
-    // can take a while.
-    std::thread([&, ThreadCount] {
-      for (size_t i = 1; i < ThreadCount; ++i) {
-        std::thread([=] { work(); }).detach();
-      }
-      work();
-    }).detach();
+  explicit ThreadPoolExecutor(
+      unsigned ThreadCount = std::thread::hardware_concurrency()) {
+    Pool.reserve(ThreadCount);
+    for (unsigned i = 0; i < ThreadCount; ++i) {
+      Pool.push_back(std::thread([=] { work(); }));
+    }
   }
 
   ~ThreadPoolExecutor() override {
@@ -53,7 +48,9 @@
     Stop = true;
     Lock.unlock();
     Cond.notify_all();
-    // Wait for ~Latch.
+    for (auto &Thread : Pool) {
+      Thread.join();
+    }
   }
 
   void add(std::function<void()> F) override {
@@ -75,14 +72,13 @@
       Lock.unlock();
       Task();
     }
-    Done.dec();
   }
 
   std::atomic<bool> Stop{false};
   std::stack<std::function<void()>> WorkStack;
   std::mutex Mutex;
   std::condition_variable Cond;
-  parallel::detail::Latch Done;
+  std::vector<std::thread> Pool;
 };
 
 Executor *Executor::getDefaultExecutor() {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72090.236025.patch
Type: text/x-patch
Size: 1697 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200103/5f443166/attachment.bin>