From 56dde3df54ae3b2f6fa186e583e13605a668064a Mon Sep 17 00:00:00 2001 From: Tim Ledbetter Date: Sun, 30 Apr 2023 17:05:11 +0100 Subject: [PATCH] LibChess+ChessEngine: Don't crash on error when reading UCI commands ChessEngine and the chess GUI will no longer crash on error while reading UCI commands. ChessEngine will print a message to the standard output, while the GUI will ignore unknown commands. Both will print the error to the debug log if the UCI_DEBUG flag is enabled. Trailing and preceding whitespace is now stripped from commands before they are parsed. Commands which are just whitespace no longer produce errors. --- Userland/Libraries/LibChess/UCIEndpoint.cpp | 54 +++++++++++++-------- Userland/Libraries/LibChess/UCIEndpoint.h | 5 +- Userland/Services/ChessEngine/ChessEngine.h | 5 +- 3 files changed, 41 insertions(+), 23 deletions(-) diff --git a/Userland/Libraries/LibChess/UCIEndpoint.cpp b/Userland/Libraries/LibChess/UCIEndpoint.cpp index c4ab15610e..a990810ba6 100644 --- a/Userland/Libraries/LibChess/UCIEndpoint.cpp +++ b/Userland/Libraries/LibChess/UCIEndpoint.cpp @@ -1,5 +1,6 @@ /* * Copyright (c) 2020, the SerenityOS developers. + * Copyright (c) 2023, Tim Ledbetter * * SPDX-License-Identifier: BSD-2-Clause */ @@ -80,49 +81,60 @@ void Endpoint::set_in_notifier() return; } - while (m_in->can_read_line()) - Core::EventLoop::current().post_event(*this, read_command()); + while (m_in->can_read_line()) { + auto line = m_in->read_line(4096).trim_whitespace(); + if (line.is_empty()) + continue; + + auto maybe_command = read_command(line); + if (maybe_command.is_error()) { + dbgln_if(UCI_DEBUG, "{} Error while parsing UCI command: {}, error: {}", class_name(), maybe_command.error(), line); + if (on_command_read_error) + on_command_read_error(move(line), maybe_command.release_error()); + + continue; + } + + Core::EventLoop::current().post_event(*this, maybe_command.release_value()); + } }; } -NonnullOwnPtr Endpoint::read_command() +ErrorOr> Endpoint::read_command(StringView line) const { - DeprecatedString line(ReadonlyBytes(m_in->read_line(4096).bytes()), Chomp); - dbgln_if(UCI_DEBUG, "{} Received UCI Command: {}", class_name(), line); if (line == "uci") { - return UCICommand::from_string(line).release_value_but_fixme_should_propagate_errors(); + return UCICommand::from_string(line); } else if (line.starts_with("debug"sv)) { - return DebugCommand::from_string(line).release_value_but_fixme_should_propagate_errors(); + return DebugCommand::from_string(line); } else if (line.starts_with("isready"sv)) { - return IsReadyCommand::from_string(line).release_value_but_fixme_should_propagate_errors(); + return IsReadyCommand::from_string(line); } else if (line.starts_with("setoption"sv)) { - return SetOptionCommand::from_string(line).release_value_but_fixme_should_propagate_errors(); + return SetOptionCommand::from_string(line); } else if (line.starts_with("position"sv)) { - return PositionCommand::from_string(line).release_value_but_fixme_should_propagate_errors(); + return PositionCommand::from_string(line); } else if (line.starts_with("go"sv)) { - return GoCommand::from_string(line).release_value_but_fixme_should_propagate_errors(); + return GoCommand::from_string(line); } else if (line.starts_with("stop"sv)) { - return StopCommand::from_string(line).release_value_but_fixme_should_propagate_errors(); + return StopCommand::from_string(line); } else if (line.starts_with("id"sv)) { - return IdCommand::from_string(line).release_value_but_fixme_should_propagate_errors(); + return IdCommand::from_string(line); } else if (line.starts_with("uciok"sv)) { - return UCIOkCommand::from_string(line).release_value_but_fixme_should_propagate_errors(); + return UCIOkCommand::from_string(line); } else if (line.starts_with("readyok"sv)) { - return ReadyOkCommand::from_string(line).release_value_but_fixme_should_propagate_errors(); + return ReadyOkCommand::from_string(line); } else if (line.starts_with("bestmove"sv)) { - return BestMoveCommand::from_string(line).release_value_but_fixme_should_propagate_errors(); + return BestMoveCommand::from_string(line); } else if (line.starts_with("info"sv)) { - return InfoCommand::from_string(line).release_value_but_fixme_should_propagate_errors(); + return InfoCommand::from_string(line); } else if (line.starts_with("quit"sv)) { - return QuitCommand::from_string(line).release_value_but_fixme_should_propagate_errors(); + return QuitCommand::from_string(line); } else if (line.starts_with("ucinewgame"sv)) { - return UCINewGameCommand::from_string(line).release_value_but_fixme_should_propagate_errors(); + return UCINewGameCommand::from_string(line); } - dbgln("command line: {}", line); - VERIFY_NOT_REACHED(); + return Error::from_string_literal("Unknown command"); } }; diff --git a/Userland/Libraries/LibChess/UCIEndpoint.h b/Userland/Libraries/LibChess/UCIEndpoint.h index 0ee5e1330d..87f3e00a16 100644 --- a/Userland/Libraries/LibChess/UCIEndpoint.h +++ b/Userland/Libraries/LibChess/UCIEndpoint.h @@ -1,5 +1,6 @@ /* * Copyright (c) 2020-2022, the SerenityOS developers. + * Copyright (c) 2023, Tim Ledbetter * * SPDX-License-Identifier: BSD-2-Clause */ @@ -18,6 +19,8 @@ class Endpoint : public Core::Object { public: virtual ~Endpoint() override = default; + Function on_command_read_error; + virtual void handle_uci() { } virtual void handle_debug(DebugCommand const&) { } virtual void handle_isready() { } @@ -58,7 +61,7 @@ private: UnexpectedEof }; void set_in_notifier(); - NonnullOwnPtr read_command(); + ErrorOr> read_command(StringView line) const; RefPtr m_in; RefPtr m_out; diff --git a/Userland/Services/ChessEngine/ChessEngine.h b/Userland/Services/ChessEngine/ChessEngine.h index 9afc0fc394..5422878843 100644 --- a/Userland/Services/ChessEngine/ChessEngine.h +++ b/Userland/Services/ChessEngine/ChessEngine.h @@ -1,5 +1,6 @@ /* * Copyright (c) 2020, the SerenityOS developers. + * Copyright (c) 2023, Tim Ledbetter * * SPDX-License-Identifier: BSD-2-Clause */ @@ -25,10 +26,12 @@ public: Function on_quit; private: - ChessEngine() = default; ChessEngine(NonnullRefPtr in, NonnullRefPtr out) : Endpoint(in, out) { + on_command_read_error = [](auto command, auto error) { + outln("{}: '{}'", error, command); + }; } Chess::Board m_board;