Fix a race in idle_loop() exiting

[stockfish] / src / search.cpp

diff --git a/src/search.cpp b/src/search.cpp
index 71b974b677d893f3325734f0de639de984c9e2b5..7ba64c44337dbe3de43276342f89670d03ed64ba 100644 (file)
--- a/src/search.cpp
+++ b/src/search.cpp
@@ -559,6 +559,7 @@ bool think(const Position& pos, bool infinite, bool ponder, int side_to_move,
 void init_threads() {
 
   volatile int i;
+  bool ok;
 
 #if !defined(_MSC_VER)
   pthread_t pthread[1];
@@ -598,12 +599,18 @@ void init_threads() {
   for (i = 1; i < THREAD_MAX; i++)
   {
 #if !defined(_MSC_VER)
-      pthread_create(pthread, NULL, init_thread, (void*)(&i));
+      ok = (pthread_create(pthread, NULL, init_thread, (void*)(&i)) == 0);
 #else
       DWORD iID[1];
-      CreateThread(NULL, 0, init_thread, (LPVOID)(&i), 0, iID);
+      ok = (CreateThread(NULL, 0, init_thread, (LPVOID)(&i), 0, iID) != NULL);
 #endif
 
+      if (!ok)
+      {
+          cout << "Failed to create thread number " << i << endl;
+          Application::exit_with_failure();
+      }
+
       // Wait until the thread has finished launching
       while (!Threads[i].running);
   }
@@ -1806,7 +1813,7 @@ namespace {
     assert(threadID >= 0 && threadID < ActiveThreads);
     assert(ActiveThreads > 1);
 
-    Position pos = Position(sp->pos);
+    Position pos(*sp->pos);
     CheckInfo ci(pos);
     SearchStack* ss = sp->sstack[threadID];
     Value value = -VALUE_INFINITE;
@@ -1948,7 +1955,7 @@ namespace {
     assert(threadID >= 0 && threadID < ActiveThreads);
     assert(ActiveThreads > 1);
 
-    Position pos = Position(sp->pos);
+    Position pos(*sp->pos);
     CheckInfo ci(pos);
     SearchStack* ss = sp->sstack[threadID];
     Value value = -VALUE_INFINITE;
@@ -2796,6 +2803,8 @@ namespace {
       // If this thread has been assigned work, launch a search
       if (Threads[threadID].workIsWaiting)
       {
+          assert(!Threads[threadID].idle);
+
           Threads[threadID].workIsWaiting = false;
           if (Threads[threadID].splitPoint->pvNode)
               sp_search_pv(Threads[threadID].splitPoint, threadID);
@@ -2941,7 +2950,6 @@ namespace {
     assert(ActiveThreads > 1);
 
     SplitPoint* splitPoint;
-    int i;
 
     lock_grab(&MPLock);
 
@@ -2958,7 +2966,7 @@ namespace {
     splitPoint = SplitPointStack[master] + Threads[master].activeSplitPoints;
     Threads[master].activeSplitPoints++;
 
-    // Initialize the split point object and copy current position
+    // Initialize the split point object
     splitPoint->parent = Threads[master].splitPoint;
     splitPoint->finished = false;
     splitPoint->ply = ply;
@@ -2972,36 +2980,45 @@ namespace {
     splitPoint->mp = mp;
     splitPoint->moves = *moves;
     splitPoint->cpus = 1;
-    splitPoint->pos.copy(p);
+    splitPoint->pos = &p;
     splitPoint->parentSstack = sstck;
-    for (i = 0; i < ActiveThreads; i++)
+    for (int i = 0; i < ActiveThreads; i++)
         splitPoint->slaves[i] = 0;
 
-    // Copy the current search stack to the master thread
-    memcpy(splitPoint->sstack[master], sstck, (ply+1) * sizeof(SearchStack));
+    Threads[master].idle = false;
     Threads[master].splitPoint = splitPoint;
 
-    // Make copies of the current position and search stack for each thread
-    for (i = 0; i < ActiveThreads && splitPoint->cpus < MaxThreadsPerSplitPoint; i++)
+    // Allocate available threads setting idle flag to false
+    for (int i = 0; i < ActiveThreads && splitPoint->cpus < MaxThreadsPerSplitPoint; i++)
         if (thread_is_available(i, master))
         {
-            memcpy(splitPoint->sstack[i], sstck, (ply+1) * sizeof(SearchStack));
+            Threads[i].idle = false;
             Threads[i].splitPoint = splitPoint;
             splitPoint->slaves[i] = 1;
             splitPoint->cpus++;
         }
 
+    assert(splitPoint->cpus > 1);
+
+    // We can release the lock because master and slave threads are already booked
+    lock_release(&MPLock);
+
+    // Copy the tail of current search stack to the master thread
+    memcpy(splitPoint->sstack[master] + ply - 1, sstck + ply - 1, 3 * sizeof(SearchStack));
+
     // Tell the threads that they have work to do. This will make them leave
-    // their idle loop.
-    for (i = 0; i < ActiveThreads; i++)
+    // their idle loop. Also copy search stack tail for each slave thread.
+    for (int i = 0; i < ActiveThreads; i++)
+    {
+        if (splitPoint->slaves[i])
+            memcpy(splitPoint->sstack[i] + ply - 1, sstck + ply - 1, 3 * sizeof(SearchStack));
+
         if (i == master || splitPoint->slaves[i])
         {
-            Threads[i].workIsWaiting = true;
-            Threads[i].idle = false;
             Threads[i].stop = false;
+            Threads[i].workIsWaiting = true; // This makes the slave to exit from idle_loop()
         }
-
-    lock_release(&MPLock);
+    }
 
     // Everything is set up. The master thread enters the idle loop, from
     // which it will instantly launch a search, because its workIsWaiting