refactor(thread_pool): don't lock notifying

chenshuo · Oct 29, 2022 · 4a6a5cf · 4a6a5cf
1 parent 11e3c42
commit 4a6a5cf
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 14 deletions.
diff --git a/thread/ThreadPool.cc b/thread/ThreadPool.cc
@@ -48,7 +48,10 @@ void ThreadPool::start(int numThreads)
 void ThreadPool::stop()
 {
   printf("starting to stop the pool.\n");
-  running_ = false;
+  {
+    MutexLockGuard lock(mutex_);
+    running_ = false;
+  }
   cond_.notifyAll();
   for_each(threads_.begin(),
            threads_.end(),
@@ -62,13 +65,13 @@ void ThreadPool::run(const Task& task)
   if (threads_.empty())
   {
     task();
+    return;
   }
-  else
   {
     MutexLockGuard lock(mutex_);
     queue_.push_back(task);
-    cond_.notify();
   }
+  cond_.notify();
 }
 
 ThreadPool::Task ThreadPool::take()

diff --git a/thread/ThreadPool.h b/thread/ThreadPool.h
@@ -38,18 +38,21 @@ class ThreadPool : boost::noncopyable
   template<typename Func, typename... Args>
   inline auto run_future(Func&& func, Args&&... args) -> std::future<typename std::result_of<Func(Args...)>::type>
   {
-      if (threads_.empty()) {
-        func(args...);
+    if (threads_.empty()) {
+      func(args...);
+    }
+    using ret_type = typename std::result_of<Func(Args...)>::type;
+    auto task = std::make_shared<std::packaged_task<ret_type()>>(std::bind(std::forward<Func>(func), std::forward<Args>(args)...));
+    auto ret = task->get_future();
+    do {
+      MutexLockGuard lock(mutex_);
+      if (!running_) {
+        throw std::runtime_error("enqueue on stopped ThreadPool");
       }
-      using ret_type = typename std::result_of<Func(Args...)>::type;
-      auto task = std::make_shared<std::packaged_task<ret_type()>>(std::bind(std::forward<Func>(func), std::forward<Args>(args)...));
-      auto ret = task->get_future();
-      do {
-          MutexLockGuard lock(mutex_);
-          queue_.push_back([task]() { (*task)(); });
-          cond_.notify();
-      } while (0);
-      return std::move(ret);
+      queue_.push_back([task]() { (*task)(); });
+    } while (0);
+    cond_.notify();
+    return std::move(ret);
   }
 
  private: