From 275787d918d173124d4358e669838d0b4e68db0b Mon Sep 17 00:00:00 2001 From: Fathzer Date: Wed, 28 Feb 2024 10:10:47 +0100 Subject: [PATCH] Starts better bot's process failure management --- .gitignore | 1 + src/main/java/com/fathzer/jchess/Game.java | 58 ++++++------ .../java/com/fathzer/jchess/bot/Engine.java | 11 ++- .../fathzer/jchess/bot/uci/EngineLoader.java | 3 +- .../com/fathzer/jchess/bot/uci/UCIEngine.java | 92 ++++++++++++------- .../jchess/internal/InternalEngine.java | 8 +- .../com/fathzer/jchess/swing/GameSession.java | 11 ++- .../jchess/swing/settings/EnginesPanel.java | 2 +- .../com/fathzer/util/ProcessExitDetector.java | 41 +++++++++ 9 files changed, 155 insertions(+), 72 deletions(-) create mode 100644 src/main/java/com/fathzer/util/ProcessExitDetector.java diff --git a/.gitignore b/.gitignore index 8c0e321..89dfe28 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ /.project /.settings/ /dependency-reduced-pom.xml +/data/pgn diff --git a/src/main/java/com/fathzer/jchess/Game.java b/src/main/java/com/fathzer/jchess/Game.java index bd39214..e98e671 100644 --- a/src/main/java/com/fathzer/jchess/Game.java +++ b/src/main/java/com/fathzer/jchess/Game.java @@ -1,8 +1,10 @@ package com.fathzer.jchess; +import java.io.IOException; import java.util.concurrent.Executor; import java.util.concurrent.Executors; import java.util.function.BiConsumer; +import java.util.function.Consumer; import com.fathzer.jchess.bot.Engine; import com.fathzer.jchess.fen.FENUtils; @@ -21,35 +23,6 @@ public class Game { return t; }); - private class EngineTurn implements Runnable { - private final Engine engine; - private final BiConsumer moveConsumer; - - private EngineTurn(Engine engine, BiConsumer moveConsumer) { - this.moveConsumer = moveConsumer; - this.engine = engine; - } - - @Override - public void run() { - final CoordinatesSystem cs = board.getCoordinatesSystem(); - engine.setPosition(FENUtils.to(history.getStartBoard()), history.getMoves().stream().map(m -> JChessUCIEngine.toUCIMove(cs, m)). - map(UCIMove::toString).toList()); - final CountDownState params; - if (clock==null) { - params = null; - } else { - final long remainingTime = clock.getRemaining(clock.getPlaying()); - final ClockSettings clockSettings = clock.getCurrentSettings(clock.getPlaying()); - final int increment = clockSettings.getIncrement()>0 ? clockSettings.getIncrement()/clockSettings.getMovesNumberBeforeIncrement() : 0; - final int movesToGo = clock.getRemainingMovesBeforeNext(clock.getPlaying()); - params = new CountDownState(remainingTime, increment, movesToGo); - } - Move move = JChessUCIEngine.toMove(board, UCIMove.from(engine.play(params))); - moveConsumer.accept(Game.this, move); - } - } - @Getter private final Board board; @Getter @@ -94,9 +67,32 @@ public void pause() { } } } + + private Move getMove(Engine engine) throws IOException { + final CoordinatesSystem cs = board.getCoordinatesSystem(); + engine.setPosition(FENUtils.to(history.getStartBoard()), history.getMoves().stream().map(m -> JChessUCIEngine.toUCIMove(cs, m)). + map(UCIMove::toString).toList()); + final CountDownState params; + if (clock==null) { + params = null; + } else { + final long remainingTime = clock.getRemaining(clock.getPlaying()); + final ClockSettings clockSettings = clock.getCurrentSettings(clock.getPlaying()); + final int increment = clockSettings.getIncrement()>0 ? clockSettings.getIncrement()/clockSettings.getMovesNumberBeforeIncrement() : 0; + final int movesToGo = clock.getRemainingMovesBeforeNext(clock.getPlaying()); + params = new CountDownState(remainingTime, increment, movesToGo); + } + return JChessUCIEngine.toMove(board, UCIMove.from(engine.getMove(params))); + } - public void playEngine(Engine engine, BiConsumer moveConsumer) { - EXECUTOR.execute(new EngineTurn(engine, moveConsumer)); + public void playEngine(Engine engine, BiConsumer moveConsumer, Consumer errorManager) { + EXECUTOR.execute(() -> { + try { + moveConsumer.accept(this, getMove(engine)); + } catch (IOException e) { + errorManager.accept(e); + } + }); } public void onMove(Move move) { diff --git a/src/main/java/com/fathzer/jchess/bot/Engine.java b/src/main/java/com/fathzer/jchess/bot/Engine.java index d27e6bf..948b206 100644 --- a/src/main/java/com/fathzer/jchess/bot/Engine.java +++ b/src/main/java/com/fathzer/jchess/bot/Engine.java @@ -1,12 +1,15 @@ package com.fathzer.jchess.bot; import java.io.Closeable; +import java.io.IOException; import java.util.List; import com.fathzer.games.clock.CountDownState; import com.fathzer.jchess.settings.GameSettings.Variant; public interface Engine extends Closeable { + String getName(); + List> getOptions(); /** Tests whether a variant is supported or not by this engine. @@ -18,14 +21,16 @@ public interface Engine extends Closeable { /** Starts a new game. * @param variant The game variant * @return false if the variant is not supported + * @throws IOException If communication with engine fails */ - boolean newGame(Variant variant); + boolean newGame(Variant variant) throws IOException; - void setPosition(String fen, List moves); + void setPosition(String fen, List moves) throws IOException; /** Gets the engine move choice. * @param params The current engine's clock count down (null if no clock is set) * @return a Move in UCI format + * @throws IOException If communication with engine fails */ - String play(CountDownState params); + String getMove(CountDownState params) throws IOException; } diff --git a/src/main/java/com/fathzer/jchess/bot/uci/EngineLoader.java b/src/main/java/com/fathzer/jchess/bot/uci/EngineLoader.java index 760444c..9bbc8ef 100644 --- a/src/main/java/com/fathzer/jchess/bot/uci/EngineLoader.java +++ b/src/main/java/com/fathzer/jchess/bot/uci/EngineLoader.java @@ -36,7 +36,8 @@ public static void init() throws IOException { if (data!=null) { return; } - final EngineData internal = new EngineData("JChess", null, new InternalEngine()); + final InternalEngine engine = new InternalEngine(); + final EngineData internal = new EngineData(engine.getName(), null, engine); final EngineData[] array; IOException error = null; if (!Files.exists(PATH)) { diff --git a/src/main/java/com/fathzer/jchess/bot/uci/UCIEngine.java b/src/main/java/com/fathzer/jchess/bot/uci/UCIEngine.java index d73b7ee..bed2550 100644 --- a/src/main/java/com/fathzer/jchess/bot/uci/UCIEngine.java +++ b/src/main/java/com/fathzer/jchess/bot/uci/UCIEngine.java @@ -2,6 +2,7 @@ import java.io.BufferedReader; import java.io.BufferedWriter; +import java.io.EOFException; import java.io.IOException; import java.io.UncheckedIOException; import java.util.ArrayList; @@ -17,6 +18,7 @@ import com.fathzer.jchess.bot.Option.Type; import com.fathzer.jchess.bot.uci.EngineLoader.EngineData; import com.fathzer.jchess.settings.GameSettings.Variant; +import com.fathzer.util.ProcessExitDetector; import lombok.extern.slf4j.Slf4j; @@ -34,15 +36,30 @@ public class UCIEngine implements Engine { private boolean is960Supported; private boolean whiteToPlay; private boolean positionSet; + private boolean expectedRunning; public UCIEngine(EngineData data) throws IOException { log.info ("Launching process {}", Arrays.asList(data.getCommand())); this.data = data; final ProcessBuilder processBuilder = new ProcessBuilder(data.getCommand()); this.process = processBuilder.start(); + this.expectedRunning = true; this.reader = process.inputReader(); this.writer = process.outputWriter(); this.errorReader = new StdErrReader(process); + new ProcessExitDetector(process, p -> { + if (expectedRunning) { + expectedRunning = false; + log.warn("{} UCI engine exited unexpectedly with code {}", data.getName(), p.exitValue()); + try { + closeStreams(); + } catch (IOException e) { + log.error("The following error occured while closing streams of "+data.getName()+" UCI engine"); + } + } else { + log.info("{} UCI engine exited with code {}", data.getName(), p.exitValue()); + } + }).start(true); this.options = new ArrayList<>(); init(); } @@ -53,7 +70,7 @@ private void init() throws IOException { do { line = read(); if (line==null) { - break; + throw new EOFException(); } final Optional> ooption = parseOption(line); if (ooption.isPresent()) { @@ -62,7 +79,13 @@ private void init() throws IOException { is960Supported = true; } else if (!PONDER_OPTION.equals(option.getName())) { // Ponder is not supported yet - option.addListener((prev, cur) -> setOption(option, cur)); + option.addListener((prev, cur) -> { + try { + setOption(option, cur); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); options.add(option); } } @@ -77,7 +100,7 @@ private Optional> parseOption(String line) throws IOException { } } - private void setOption(Option option, Object value) { + private void setOption(Option option, Object value) throws IOException { final StringBuilder buf = new StringBuilder("setoption name "); buf.append(option.getName()); if (Type.BUTTON!=option.getType()) { @@ -87,24 +110,16 @@ private void setOption(Option option, Object value) { write(buf.toString()); } - private void write(String line) { - try { - this.writer.write(line); - this.writer.newLine(); - this.writer.flush(); - log.info(">{}: {}", data.getName(), line); - } catch (IOException e) { - throw new UncheckedIOException(e); - } + private void write(String line) throws IOException { + this.writer.write(line); + this.writer.newLine(); + this.writer.flush(); + log.info(">{}: {}", data.getName(), line); } - private String read() { - try { - final String line = reader.readLine(); - log.info("<{} : {}", data.getName(), line); - return line; - } catch (IOException e) { - throw new UncheckedIOException(e); - } + private String read() throws IOException { + final String line = reader.readLine(); + log.info("<{} : {}", data.getName(), line); + return line; } @Override @@ -118,7 +133,7 @@ public boolean isSupported(Variant variant) { } @Override - public boolean newGame(Variant variant) { + public boolean newGame(Variant variant) throws IOException { positionSet = false; if (variant==Variant.CHESS960 && !is960Supported) { return false; @@ -135,18 +150,19 @@ public boolean newGame(Variant variant) { * @param answerValidator a predicate that checks the lines returned by engine. * @return The line that is considered valid, null if no valid line is returned * and the engine closed its output. + * @throws IOException If communication with engine fails */ - private String waitAnswer(Predicate answerValidator) { + private String waitAnswer(Predicate answerValidator) throws IOException { for (String line = read(); line!=null; line=read()) { if (answerValidator.test(line)) { return line; } } - return null; + throw new EOFException(); } @Override - public void setPosition(String fen, List moves) { + public void setPosition(String fen, List moves) throws IOException { whiteToPlay = "w".equals(fen.split(" ")[1]); if (moves.size()%2!=0) { whiteToPlay = !whiteToPlay; @@ -164,7 +180,7 @@ public void setPosition(String fen, List moves) { } @Override - public String play(CountDownState params) { + public String getMove(CountDownState params) throws IOException { if (!positionSet) { throw new IllegalStateException("No position defined"); } @@ -190,26 +206,34 @@ public String play(CountDownState params) { write (command.toString()); var bestMovePrefix = "bestmove "; final String answer = waitAnswer(s -> s.startsWith(bestMovePrefix)); - if (answer!=null) { - return answer.substring(bestMovePrefix.length()); - } else { - return null; - } + return answer.substring(bestMovePrefix.length()); } @Override public void close() throws IOException { + if (!expectedRunning) { + return; + } + expectedRunning = false; this.write("quit"); - this.reader.close(); - this.writer.close(); - this.errorReader.close(); + closeStreams(); try { this.process.waitFor(5, TimeUnit.SECONDS); - log.info("{} UCI engine exited", data.getName()); } catch (InterruptedException e) { log.warn("Fail to gracefully close UCI engine {}, trying to destroy it", data.getName()); this.process.destroy(); Thread.currentThread().interrupt(); } } + + private void closeStreams() throws IOException { + this.reader.close(); + this.writer.close(); + this.errorReader.close(); + } + + @Override + public String getName() { + return data.getName(); + } } diff --git a/src/main/java/com/fathzer/jchess/internal/InternalEngine.java b/src/main/java/com/fathzer/jchess/internal/InternalEngine.java index d726698..262b08a 100644 --- a/src/main/java/com/fathzer/jchess/internal/InternalEngine.java +++ b/src/main/java/com/fathzer/jchess/internal/InternalEngine.java @@ -27,6 +27,7 @@ import com.fathzer.jchess.uci.UCIMove; public class InternalEngine implements Engine { + private static final String NAME = "JChess"; private static final long MILLIS_IN_SECONDS = 1000L; private static final BasicTimeManager> TIME_MANAGER = new BasicTimeManager<>(VuckovicSolakOracle.INSTANCE); @@ -111,7 +112,7 @@ public void setPosition(String startpos, List moves) { } @Override - public String play(CountDownState countDownState) { + public String getMove(CountDownState countDownState) { if (board==null) { throw new IllegalStateException("No position set"); } @@ -130,4 +131,9 @@ public String play(CountDownState countDownState) { public void close() { // Nothing to do } + + @Override + public String getName() { + return NAME; + } } diff --git a/src/main/java/com/fathzer/jchess/swing/GameSession.java b/src/main/java/com/fathzer/jchess/swing/GameSession.java index af8a690..d9364cd 100644 --- a/src/main/java/com/fathzer/jchess/swing/GameSession.java +++ b/src/main/java/com/fathzer/jchess/swing/GameSession.java @@ -173,7 +173,10 @@ private void nextMove() { panel.getBoard().setManualMoveEnabled(engine==null); if (engine!=null) { log.debug("Engine detected for {}",activeColor); - game.playEngine(engine, this::play); + game.playEngine(engine, this::play, e -> { + log.error("Error while communicating with "+engine.getName()+" engine", e); + SwingUtilities.invokeLater(() -> onEngineError(engine)); + }); } } @@ -246,6 +249,12 @@ private void resign(Color color) { setState(State.RUNNING); } } + + private void onEngineError(Engine engine) { + JOptionPane.showMessageDialog(panel, "An error occured while communicating with the "+engine.getName()+" engine. Assuming it resigns", "Error", JOptionPane.ERROR_MESSAGE); + final Status status = Color.WHITE.equals(game.getBoard().getActiveColor()) ? Status.BLACK_WON : Status.WHITE_WON; + endOfGame(status); + } private void endOfGame(final Status status) { setState(State.PAUSED); diff --git a/src/main/java/com/fathzer/jchess/swing/settings/EnginesPanel.java b/src/main/java/com/fathzer/jchess/swing/settings/EnginesPanel.java index 02b565d..29fda37 100644 --- a/src/main/java/com/fathzer/jchess/swing/settings/EnginesPanel.java +++ b/src/main/java/com/fathzer/jchess/swing/settings/EnginesPanel.java @@ -172,7 +172,7 @@ private void doStart() { firePropertyChange(STARTED_PROPERTY_NAME, null, engine); refreshLists(); } catch (IOException e) { - JOptionPane.showMessageDialog(Utils.getOwnerWindow(this), "An exception occurred while startting "+engine.getName()+" engine", "Error", JOptionPane.ERROR_MESSAGE); + JOptionPane.showMessageDialog(Utils.getOwnerWindow(this), "An error occurred while starting "+engine.getName()+" engine", "Error", JOptionPane.ERROR_MESSAGE); } } diff --git a/src/main/java/com/fathzer/util/ProcessExitDetector.java b/src/main/java/com/fathzer/util/ProcessExitDetector.java new file mode 100644 index 0000000..fe5a3f4 --- /dev/null +++ b/src/main/java/com/fathzer/util/ProcessExitDetector.java @@ -0,0 +1,41 @@ +package com.fathzer.util; + +import java.util.function.Consumer; + +/** + * Detects when a process is finished and invokes the associated listeners. + */ +public class ProcessExitDetector implements Runnable { + + /** The process for which we have to detect the end. */ + private Process process; + /** The associated listeners to be invoked at the end of the process. */ + private Consumer listener; + + /** + * Starts the detection for the given process + * @param process the process for which we have to detect when it is finished + */ + public ProcessExitDetector(Process process, Consumer listener) { + this.process = process; + this.listener = listener; + } + + @Override + public void run() { + try { + // wait for the process to finish + process.waitFor(); + // invokes the listener + listener.accept(process); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + public void start(boolean daemon) { + final Thread thread = new Thread(this); + thread.setDaemon(daemon); + thread.start(); + } +} \ No newline at end of file