Fix incorrect draw detection
authorMarco Costalba <mcostalba@gmail.com>
Sun, 17 Apr 2016 19:31:19 +0000 (21:31 +0200)
committerMarco Costalba <mcostalba@gmail.com>
Sun, 17 Apr 2016 22:13:16 +0000 (00:13 +0200)
In this position we should have draw for repetition:

position fen rnbqkbnr/2pppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1 moves g1f3 g8f6 f3g1
go infinite

But latest patch broke it.

Actually we had two(!) very subtle bugs, the first is that Position::set()
clears the passed state and in particular 'previous' member, so
that on passing setupStates, 'previous' pointer was reset.

Second bug is even more subtle: SetupStates was based on std::vector
as container, but when vector grows, std::vector copies all its contents
to a new location invalidating all references to its entries. Because
all StateInfo records are linked by 'previous' pointer, this made pointers
go stale upon adding more element to setupStates. So revert to use a
std::deque that ensures references are preserved when pushing back new
elements.

No functional change.

src/benchmark.cpp
src/position.h
src/thread.cpp
src/uci.cpp

index 2978f3468ff55812c6c0551499638d8c7ebe67a4..95125404222ddf9d481bf3775ddd42108a8ff7d8 100644 (file)
@@ -148,7 +148,7 @@ void benchmark(const Position& current, istream& is) {
 
   for (size_t i = 0; i < fens.size(); ++i)
   {
-      StateListPtr states(new std::vector<StateInfo>(1));
+      StateListPtr states(new std::deque<StateInfo>(1));
       pos.set(fens[i], Options["UCI_Chess960"], &states->back(), Threads.main());
 
       cerr << "\nPosition: " << i + 1 << '/' << fens.size() << endl;
index d44ed009e5d2405d8cb0826241b936724e6df488..c3ba5ac8a5b98a32c5a81099fc0519ce5eeede23 100644 (file)
@@ -23,6 +23,7 @@
 
 #include <cassert>
 #include <cstddef>  // For offsetof()
+#include <deque>
 #include <memory>   // For std::unique_ptr
 #include <string>
 #include <vector>
@@ -77,7 +78,8 @@ struct StateInfo {
   StateInfo* previous;
 };
 
-typedef std::unique_ptr<std::vector<StateInfo>> StateListPtr;
+// In a std::deque references to elements are unaffected upon resizing
+typedef std::unique_ptr<std::deque<StateInfo>> StateListPtr;
 
 
 /// Position class stores information regarding the board representation as
index dc4ec05e2f586607453270181e6cd9e7bbe841d6..4dc7d9e9b02aa565a56dd626155c706baf5893ad 100644 (file)
@@ -190,6 +190,8 @@ void ThreadPool::start_thinking(const Position& pos, StateListPtr& states,
   if (states.get())
       setupStates = std::move(states); // Ownership transfer, states is now empty
 
+  StateInfo tmp = setupStates->back();
+
   for (Thread* th : Threads)
   {
       th->maxPly = 0;
@@ -198,5 +200,7 @@ void ThreadPool::start_thinking(const Position& pos, StateListPtr& states,
       th->rootPos.set(pos.fen(), pos.is_chess960(), &setupStates->back(), th);
   }
 
+  setupStates->back() = tmp; // Restore st->previous, cleared by Position::set()
+
   main()->start_searching();
 }
index cfd253b1f586a5c8d7ddda7f50bb8b5b8162eeac..a9811a9af7352ae74308979a4751721b6bfcc042 100644 (file)
@@ -42,7 +42,7 @@ namespace {
   // A list to keep track of the position states along the setup moves (from the
   // start position to the position just before the search starts). Needed by
   // 'draw by repetition' detection.
-  StateListPtr States(new std::vector<StateInfo>(1));
+  StateListPtr States(new std::deque<StateInfo>(1));
 
 
   // position() is called when engine receives the "position" UCI command.
@@ -68,7 +68,7 @@ namespace {
     else
         return;
 
-    States = StateListPtr(new std::vector<StateInfo>(1));
+    States = StateListPtr(new std::deque<StateInfo>(1));
     pos.set(fen, Options["UCI_Chess960"], &States->back(), Threads.main());
 
     // Parse move list (if any)