From 3df2c01b5769c7ae996fb5b992c06e4a5428ad35 Mon Sep 17 00:00:00 2001 From: Marco Costalba Date: Mon, 27 Aug 2012 14:39:59 +0200 Subject: [PATCH 1/1] 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 --- src/benchmark.cpp | 3 ++- src/search.cpp | 1 + src/search.h | 7 ++++++- src/thread.cpp | 3 ++- src/thread.h | 4 ++-- src/uci.cpp | 17 ++++++++--------- 6 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/benchmark.cpp b/src/benchmark.cpp index c3a41913..9dc103ed 100644 --- a/src/benchmark.cpp +++ b/src/benchmark.cpp @@ -110,6 +110,7 @@ void benchmark(const Position& current, istream& is) { } int64_t nodes = 0; + Search::StateStackPtr st; 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 { - Threads.start_searching(pos, limits, vector()); + Threads.start_searching(pos, limits, vector(), st); Threads.wait_for_search_finished(); nodes += Search::RootPosition.nodes_searched(); } diff --git a/src/search.cpp b/src/search.cpp index b86b9559..3309947e 100644 --- a/src/search.cpp +++ b/src/search.cpp @@ -43,6 +43,7 @@ namespace Search { std::vector RootMoves; Position RootPosition; Time SearchTime; + StateStackPtr SetupStates; } using std::string; diff --git a/src/search.h b/src/search.h index d580b546..63b9b326 100644 --- a/src/search.h +++ b/src/search.h @@ -21,12 +21,14 @@ #define SEARCH_H_INCLUDED #include +#include +#include #include #include "misc.h" +#include "position.h" #include "types.h" -class Position; struct SplitPoint; namespace Search { @@ -91,11 +93,14 @@ struct SignalsType { bool stopOnPonderhit, firstRootMove, stop, failedLowAtRoot; }; +typedef std::auto_ptr > StateStackPtr; + extern volatile SignalsType Signals; extern LimitsType Limits; extern std::vector RootMoves; extern Position RootPosition; extern Time SearchTime; +extern StateStackPtr SetupStates; extern void init(); extern size_t perft(Position& pos, Depth depth); diff --git a/src/thread.cpp b/src/thread.cpp index ce2f41f2..541610a1 100644 --- 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, - const std::vector& searchMoves) { + const std::vector& searchMoves, StateStackPtr& states) { 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; + SetupStates = states; // Ownership transfer here RootMoves.clear(); for (MoveList ml(pos); !ml.end(); ++ml) diff --git a/src/thread.h b/src/thread.h index 5feab000..6a929a98 100644 --- 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(); - void start_searching(const Position& pos, const Search::LimitsType& limits, - const std::vector& searchMoves); + void start_searching(const Position&, const Search::LimitsType&, + const std::vector&, Search::StateStackPtr&); template 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 ac58b43c..31d0a721 100644 --- a/src/uci.cpp +++ b/src/uci.cpp @@ -17,7 +17,6 @@ along with this program. If not, see . */ -#include #include #include #include @@ -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"; + // 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); @@ -155,10 +158,6 @@ namespace { 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 st; - Move m; string token, fen; @@ -176,13 +175,13 @@ namespace { return; pos.from_fen(fen, Options["UCI_Chess960"], Threads.main_thread()); - st.clear(); + SetupStates = Search::StateStackPtr(new std::stack()); // 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)); } - Threads.start_searching(pos, limits, searchMoves); + Threads.start_searching(pos, limits, searchMoves, SetupStates); } } -- 2.39.2