Correctly handle handover of setup states

Before the search we setup the starting position doing all the
moves (sent by GUI) from start position to the position just
before to start searching.

To do this we use a set of StateInfo records used by each
do_move() call. These records shall be kept valid during all
the search because repetition draw detection uses them to back
track all the earlier positions keys. The problem is that, while
searching, the GUI could send another 'position' command, this
calls set_position() that clears the states! Of course a crash
follows shortly.

Before searching all the relevant parameters are copied in
start_searching() just for this reason: to fully detach data
accessed during the search from the UCI protocol handling.
So the natural solution would be to copy also the setup states.
Unfortunatly this approach does not work because StateInfo
contains a pointer to the previous record, so naively copying and
then freeing the original memory leads to a crash.

That's why we use two std::auto_ptr (one belonging to UCI and another
to Search) to safely transfer ownership of the StateInfo records to
the search, after we have setup the root position.

As a nice side-effect all the possible memory leaks are magically
sorted out for us by std::auto_ptr semantic.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

diff --git a/src/benchmark.cpp b/src/benchmark.cpp
index c3a4191337d69c4d8f91f637e2c3839b8857eb78..9dc103ed1992681b0fdef8252d3cc36cd73cb140 100644 (file)
--- a/src/benchmark.cpp
+++ b/src/benchmark.cpp
@@ -110,6 +110,7 @@ void benchmark(const Position& current, istream& is) {
   }
 
   int64_t nodes = 0;
   }
 
   int64_t nodes = 0;

+  Search::StateStackPtr st;

   Time time = Time::current_time();
 
   for (size_t i = 0; i < fens.size(); i++)
   Time time = Time::current_time();
 
   for (size_t i = 0; i < fens.size(); i++)


@@ -126,7 +127,7 @@ void benchmark(const Position& current, istream& is) {
       }
       else
       {
       }
       else
       {

-          Threads.start_searching(pos, limits, vector<Move>());
+          Threads.start_searching(pos, limits, vector<Move>(), st);

           Threads.wait_for_search_finished();
           nodes += Search::RootPosition.nodes_searched();
       }
           Threads.wait_for_search_finished();
           nodes += Search::RootPosition.nodes_searched();
       }

diff --git a/src/search.cpp b/src/search.cpp
index b86b955942624d23d9f892c9ce8b92a5dd021cc8..3309947e1c20f91b1870adde6c8a0e7c5228e504 100644 (file)
--- a/src/search.cpp
+++ b/src/search.cpp
@@ -43,6 +43,7 @@ namespace Search {
   std::vector<RootMove> RootMoves;
   Position RootPosition;
   Time SearchTime;
   std::vector<RootMove> RootMoves;
   Position RootPosition;
   Time SearchTime;

+  StateStackPtr SetupStates;

 }
 
 using std::string;
 }
 
 using std::string;

diff --git a/src/search.h b/src/search.h
index d580b54695744fe79a62e6c777adbca4ddafc61c..63b9b3264a7fb1f138bbe7e6705e119602069251 100644 (file)
--- a/src/search.h
+++ b/src/search.h
@@ -21,12 +21,14 @@
 #define SEARCH_H_INCLUDED
 
 #include <cstring>
 #define SEARCH_H_INCLUDED
 
 #include <cstring>

+#include <memory>
+#include <stack>

 #include <vector>
 
 #include "misc.h"
 #include <vector>
 
 #include "misc.h"

+#include "position.h"

 #include "types.h"
 
 #include "types.h"
 

-class Position;

 struct SplitPoint;
 
 namespace Search {
 struct SplitPoint;
 
 namespace Search {


@@ -91,11 +93,14 @@ struct SignalsType {
   bool stopOnPonderhit, firstRootMove, stop, failedLowAtRoot;
 };
 
   bool stopOnPonderhit, firstRootMove, stop, failedLowAtRoot;
 };
 

+typedef std::auto_ptr<std::stack<StateInfo> > StateStackPtr;
+

 extern volatile SignalsType Signals;
 extern LimitsType Limits;
 extern std::vector<RootMove> RootMoves;
 extern Position RootPosition;
 extern Time SearchTime;
 extern volatile SignalsType Signals;
 extern LimitsType Limits;
 extern std::vector<RootMove> RootMoves;
 extern Position RootPosition;
 extern Time SearchTime;

+extern StateStackPtr SetupStates;

 
 extern void init();
 extern size_t perft(Position& pos, Depth depth);
 
 extern void init();
 extern size_t perft(Position& pos, Depth depth);

diff --git a/src/thread.cpp b/src/thread.cpp
index ce2f41f2ffeae29008890a7917fdc3302807d827..541610a1ca4b2bd5a63d16323c4d2aed991aae21 100644 (file)
--- a/src/thread.cpp
+++ b/src/thread.cpp
@@ -414,7 +414,7 @@ void ThreadPool::wait_for_search_finished() {
 // a new search, then returns immediately.
 
 void ThreadPool::start_searching(const Position& pos, const LimitsType& limits,
 // a new search, then returns immediately.
 
 void ThreadPool::start_searching(const Position& pos, const LimitsType& limits,

-                                 const std::vector<Move>& searchMoves) {
+                                 const std::vector<Move>& searchMoves, StateStackPtr& states) {

   wait_for_search_finished();
 
   SearchTime.restart(); // As early as possible
   wait_for_search_finished();
 
   SearchTime.restart(); // As early as possible


@@ -424,6 +424,7 @@ void ThreadPool::start_searching(const Position& pos, const LimitsType& limits,
 
   RootPosition = pos;
   Limits = limits;
 
   RootPosition = pos;
   Limits = limits;

+  SetupStates = states; // Ownership transfer here

   RootMoves.clear();
 
   for (MoveList<LEGAL> ml(pos); !ml.end(); ++ml)
   RootMoves.clear();
 
   for (MoveList<LEGAL> ml(pos); !ml.end(); ++ml)

diff --git a/src/thread.h b/src/thread.h
index 5feab00072be92c2505f635c0d0800c2ca935e8c..6a929a98af1cdc44bbd9d703f5dfb597f71cfb23 100644 (file)
--- a/src/thread.h
+++ b/src/thread.h
@@ -145,8 +145,8 @@ public:
   bool available_slave_exists(Thread* master) const;
   void set_timer(int msec);
   void wait_for_search_finished();
   bool available_slave_exists(Thread* master) const;
   void set_timer(int msec);
   void wait_for_search_finished();

-  void start_searching(const Position& pos, const Search::LimitsType& limits,
-                       const std::vector<Move>& searchMoves);
+  void start_searching(const Position&, const Search::LimitsType&,
+                       const std::vector<Move>&, Search::StateStackPtr&);

 
   template <bool Fake>
   Value split(Position& pos, Search::Stack* ss, Value alpha, Value beta, Value bestValue, Move* bestMove,
 
   template <bool Fake>
   Value split(Position& pos, Search::Stack* ss, Value alpha, Value beta, Value bestValue, Move* bestMove,

diff --git a/src/uci.cpp b/src/uci.cpp
index ac58b43c2454495ff42cad4649704159b0a94139..31d0a721ab38f2038bccce775c1413bb1621f68e 100644 (file)
--- a/src/uci.cpp
+++ b/src/uci.cpp
@@ -17,7 +17,6 @@
   along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
   along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 

-#include <deque>

 #include <iostream>
 #include <sstream>
 #include <string>
 #include <iostream>
 #include <sstream>
 #include <string>


@@ -39,6 +38,10 @@ namespace {
   // FEN string of the initial position, normal chess
   const char* StartFEN = "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1";
 
   // FEN string of the initial position, normal chess
   const char* StartFEN = "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1";
 

+  // Keep track of position keys along the setup moves (from start position to the
+  // position just before to start searching). Needed by repetition draw detection.
+  Search::StateStackPtr SetupStates;
+

   void set_option(istringstream& up);
   void set_position(Position& pos, istringstream& up);
   void go(Position& pos, istringstream& up);
   void set_option(istringstream& up);
   void set_position(Position& pos, istringstream& up);
   void go(Position& pos, istringstream& up);


@@ -155,10 +158,6 @@ namespace {
 
   void set_position(Position& pos, istringstream& is) {
 
 
   void set_position(Position& pos, istringstream& is) {
 

-    // Keep track of position keys along the setup moves (from start position to the
-    // position just before to start searching). Needed by repetition draw detection.
-    static std::deque<StateInfo> st;
-

     Move m;
     string token, fen;
 
     Move m;
     string token, fen;
 


@@ -176,13 +175,13 @@ namespace {
         return;
 
     pos.from_fen(fen, Options["UCI_Chess960"], Threads.main_thread());
         return;
 
     pos.from_fen(fen, Options["UCI_Chess960"], Threads.main_thread());

-    st.clear();
+    SetupStates = Search::StateStackPtr(new std::stack<StateInfo>());

 
     // Parse move list (if any)
     while (is >> token && (m = move_from_uci(pos, token)) != MOVE_NONE)
     {
 
     // Parse move list (if any)
     while (is >> token && (m = move_from_uci(pos, token)) != MOVE_NONE)
     {

-        st.push_back(StateInfo());
-        pos.do_move(m, st.back());
+        SetupStates->push(StateInfo());
+        pos.do_move(m, SetupStates->top());

     }
   }
 
     }
   }
 


@@ -248,6 +247,6 @@ namespace {
                 searchMoves.push_back(move_from_uci(pos, token));
     }
 
                 searchMoves.push_back(move_from_uci(pos, token));
     }
 

-    Threads.start_searching(pos, limits, searchMoves);
+    Threads.start_searching(pos, limits, searchMoves, SetupStates);

   }
 }
   }
 }