From fd229c0768d80e7a71353e044556bbf74dd5c145 Mon Sep 17 00:00:00 2001
From: Tomasz Sobczyk <tomasz.sobczyk1997@gmail.com>
Date: Sun, 18 Oct 2020 22:35:16 +0200
Subject: [PATCH] Fix races and UBs

---
 src/thread.cpp | 28 ++++++++++++++++------------
 src/thread.h   | 11 +++++------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/src/thread.cpp b/src/thread.cpp
index 874b09ee..2ecd167a 100644
--- a/src/thread.cpp
+++ b/src/thread.cpp
@@ -35,6 +35,7 @@ ThreadPool Threads; // Global object
 Thread::Thread(size_t n) : idx(n), stdThread(&Thread::idle_loop, this) {
 
   wait_for_search_finished();
+  wait_for_worker_finished();
 }
 
 
@@ -80,10 +81,11 @@ void Thread::start_searching() {
   cv.notify_one(); // Wake up the thread in idle_loop()
 }
 
-void Thread::execute_task(std::function<void(Thread&)> t)
+void Thread::execute_with_worker(std::function<void(Thread&)> t)
 {
   std::lock_guard<std::mutex> lk(mutex);
-  task = std::move(t);
+  worker = std::move(t);
+  searching = true;
   cv.notify_one(); // Wake up the thread in idle_loop()
 }
 
@@ -98,10 +100,10 @@ void Thread::wait_for_search_finished() {
 }
 
 
-void Thread::wait_for_task_finished() {
+void Thread::wait_for_worker_finished() {
 
   std::unique_lock<std::mutex> lk(mutex);
-  cv.wait(lk, [&]{ return !task; });
+  cv.wait(lk, [&]{ return !searching; });
 }
 
 /// Thread::idle_loop() is where the thread is parked, blocked on the
@@ -121,18 +123,20 @@ void Thread::idle_loop() {
   {
       std::unique_lock<std::mutex> lk(mutex);
       searching = false;
+      worker = nullptr;
       cv.notify_one(); // Wake up anyone waiting for search finished
-      cv.wait(lk, [&]{ return searching || task; });
+      cv.wait(lk, [&]{ return searching; });
 
       if (exit)
           return;
 
+      auto wrk = std::move(worker);
+
       lk.unlock();
 
-      if (task)
+      if (wrk)
       {
-        task(*this);
-        task = nullptr;
+        wrk(*this);
       }
       else
       {
@@ -183,11 +187,11 @@ void ThreadPool::clear() {
 }
 
 
-void ThreadPool::execute_parallel(std::function<void(Thread&)> task)
+void ThreadPool::execute_with_workers(std::function<void(Thread&)> worker)
 {
   for(Thread* th : *this)
   {
-    th->execute_task(task);
+    th->execute_with_worker(std::move(worker));
   }
 }
 
@@ -301,8 +305,8 @@ void ThreadPool::wait_for_search_finished() const {
 }
 
 
-void ThreadPool::wait_for_tasks_finished() const {
+void ThreadPool::wait_for_workers_finished() const {
 
     for (Thread* th : *this)
-        th->wait_for_task_finished();
+        th->wait_for_worker_finished();
 }
diff --git a/src/thread.h b/src/thread.h
index 8be6eb5a..7474ea44 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -45,18 +45,19 @@ class Thread {
   std::condition_variable cv;
   size_t idx;
   bool exit = false, searching = true; // Set before starting std::thread
+  std::function<void(Thread&)> worker;
   NativeThread stdThread;
 
 public:
   explicit Thread(size_t);
   virtual ~Thread();
   virtual void search();
-  virtual void execute_task(std::function<void(Thread&)> t);
+  virtual void execute_with_worker(std::function<void(Thread&)> t);
   void clear();
   void idle_loop();
   void start_searching();
   void wait_for_search_finished();
-  void wait_for_task_finished();
+  void wait_for_worker_finished();
   size_t thread_idx() const { return idx; }
 
   Pawns::Table pawnsTable;
@@ -81,8 +82,6 @@ public:
   int Cardinality;
   bool UseRule50;
   Depth ProbeDepth;
-
-  std::function<void(Thread&)> task;
 };
 
 
@@ -110,7 +109,7 @@ struct MainThread : public Thread {
 
 struct ThreadPool : public std::vector<Thread*> {
 
-  void execute_parallel(std::function<void(Thread&)> task);
+  void execute_with_workers(std::function<void(Thread&)> worker);
 
   void start_thinking(Position&, StateListPtr&, const Search::LimitsType&, bool = false);
   void clear();
@@ -122,7 +121,7 @@ struct ThreadPool : public std::vector<Thread*> {
   Thread* get_best_thread() const;
   void start_searching();
   void wait_for_search_finished() const;
-  void wait_for_tasks_finished() const;
+  void wait_for_workers_finished() const;
 
   std::atomic_bool stop, increaseDepth;