From 002a05fef48703e20fe762f0b986179e65ca67a8 Mon Sep 17 00:00:00 2001 From: shxyke Date: Tue, 28 Mar 2023 02:02:18 +0000 Subject: [PATCH] fix: add timeout for IOHandler read --- .../Controller/MinitouchController.cpp | 29 +++-------- src/MaaCore/Controller/Platform/AdbLiteIO.cpp | 4 +- src/MaaCore/Controller/Platform/AdbLiteIO.h | 2 +- src/MaaCore/Controller/Platform/PlatformIO.h | 2 +- src/MaaCore/Controller/Platform/PosixIO.cpp | 13 ++++- src/MaaCore/Controller/Platform/PosixIO.h | 2 +- src/MaaCore/Controller/Platform/Win32IO.cpp | 50 ++++++++----------- src/MaaCore/Controller/Platform/Win32IO.h | 2 +- 8 files changed, 48 insertions(+), 56 deletions(-) diff --git a/src/MaaCore/Controller/MinitouchController.cpp b/src/MaaCore/Controller/MinitouchController.cpp index 29ca313772..0840f8e257 100644 --- a/src/MaaCore/Controller/MinitouchController.cpp +++ b/src/MaaCore/Controller/MinitouchController.cpp @@ -36,29 +36,16 @@ bool asst::MinitouchController::call_and_hup_minitouch() return false; } - auto check_timeout = [&](const auto& start_time) -> bool { - using namespace std::chrono_literals; - return std::chrono::steady_clock::now() - start_time < 3s; - }; + if (need_exit()) { + release_minitouch(true); + return false; + } - const auto start_time = std::chrono::steady_clock::now(); - while (true) { - if (need_exit()) { - release_minitouch(true); - return false; - } + pipe_str = m_minitouch_handler->read(3); - if (!check_timeout(start_time)) { - Log.info("unable to find $ from pipe_str:", Logger::separator::newline, pipe_str); - release_minitouch(true); - return false; - } - - if (pipe_str.find('$') != std::string::npos) { - break; - } - - pipe_str += m_minitouch_handler->read(); + if (pipe_str.find('$') == std::string::npos) { + release_minitouch(true); + return false; } Log.info("pipe str", Logger::separator::newline, pipe_str); diff --git a/src/MaaCore/Controller/Platform/AdbLiteIO.cpp b/src/MaaCore/Controller/Platform/AdbLiteIO.cpp index 60ca378479..6710a82bef 100644 --- a/src/MaaCore/Controller/Platform/AdbLiteIO.cpp +++ b/src/MaaCore/Controller/Platform/AdbLiteIO.cpp @@ -193,10 +193,10 @@ bool asst::IOHandlerAdbLite::write(const std::string_view data) } } -std::string asst::IOHandlerAdbLite::read() +std::string asst::IOHandlerAdbLite::read(unsigned timeout_sec) { try { - return m_handle->read(); + return m_handle->read(timeout_sec); } catch (const std::exception& e) { Log.error("IOHandler read failed:", e.what()); diff --git a/src/MaaCore/Controller/Platform/AdbLiteIO.h b/src/MaaCore/Controller/Platform/AdbLiteIO.h index 763c9d23e8..99307d57d7 100644 --- a/src/MaaCore/Controller/Platform/AdbLiteIO.h +++ b/src/MaaCore/Controller/Platform/AdbLiteIO.h @@ -50,7 +50,7 @@ namespace asst virtual ~IOHandlerAdbLite() = default; virtual bool write(const std::string_view data) override; - virtual std::string read() override; + virtual std::string read(unsigned timeout_sec) override; private: std::shared_ptr m_handle = nullptr; diff --git a/src/MaaCore/Controller/Platform/PlatformIO.h b/src/MaaCore/Controller/Platform/PlatformIO.h index be991a2f7f..a208c2acd3 100644 --- a/src/MaaCore/Controller/Platform/PlatformIO.h +++ b/src/MaaCore/Controller/Platform/PlatformIO.h @@ -40,6 +40,6 @@ namespace asst virtual ~IOHandler() = default; virtual bool write(const std::string_view data) = 0; - virtual std::string read() = 0; + virtual std::string read(unsigned timeout_sec) = 0; }; } diff --git a/src/MaaCore/Controller/Platform/PosixIO.cpp b/src/MaaCore/Controller/Platform/PosixIO.cpp index c84b6b10ba..13345caa7d 100644 --- a/src/MaaCore/Controller/Platform/PosixIO.cpp +++ b/src/MaaCore/Controller/Platform/PosixIO.cpp @@ -260,15 +260,26 @@ bool asst::IOHandlerPosix::write(const std::string_view data) return false; } -std::string asst::IOHandlerPosix::read() +std::string asst::IOHandlerPosix::read(unsigned timeout_sec) { if (m_process < 0 || m_read_fd < 0) return NULL; std::string ret_str; constexpr int PipeReadBuffSize = 4096ULL; + auto check_timeout = [&](const auto& start_time) -> bool { + using namespace std::chrono_literals; + return std::chrono::steady_clock::now() - start_time < timeout_sec * 1s; + }; + + auto start_time = std::chrono::steady_clock::now(); + while (true) { char buf_from_child[PipeReadBuffSize]; + if (!check_timeout(start_time)) { + break; + } + ssize_t ret_read = ::read(m_read_fd, buf_from_child, PipeReadBuffSize); if (ret_read > 0) { ret_str.insert(ret_str.end(), buf_from_child, buf_from_child + ret_read); diff --git a/src/MaaCore/Controller/Platform/PosixIO.h b/src/MaaCore/Controller/Platform/PosixIO.h index 5580b8cea1..7a6870214e 100644 --- a/src/MaaCore/Controller/Platform/PosixIO.h +++ b/src/MaaCore/Controller/Platform/PosixIO.h @@ -51,7 +51,7 @@ namespace asst virtual ~IOHandlerPosix(); virtual bool write(const std::string_view data) override; - virtual std::string read() override; + virtual std::string read(unsigned timeout_sec) override; private: int m_read_fd = -1; diff --git a/src/MaaCore/Controller/Platform/Win32IO.cpp b/src/MaaCore/Controller/Platform/Win32IO.cpp index 6e95fffb06..073013504b 100644 --- a/src/MaaCore/Controller/Platform/Win32IO.cpp +++ b/src/MaaCore/Controller/Platform/Win32IO.cpp @@ -281,11 +281,6 @@ std::shared_ptr asst::Win32IO::interactive_shell(const std::str constexpr int PipeReadBuffSize = 4096ULL; constexpr int PipeWriteBuffSize = 64 * 1024ULL; - auto check_timeout = [&](const auto& start_time) -> bool { - using namespace std::chrono_literals; - return std::chrono::steady_clock::now() - start_time < 3s; - }; - SECURITY_ATTRIBUTES sa_attr_inherit { .nLength = sizeof(SECURITY_ATTRIBUTES), .lpSecurityDescriptor = nullptr, @@ -329,27 +324,6 @@ std::shared_ptr asst::Win32IO::interactive_shell(const std::str return nullptr; } - auto start_time = std::chrono::steady_clock::now(); - - auto pipe_buffer = std::make_unique(PipeReadBuffSize); - OVERLAPPED pipeov { .hEvent = CreateEventW(nullptr, TRUE, FALSE, nullptr) }; - std::ignore = ReadFile(pipe_parent_read, pipe_buffer.get(), PipeReadBuffSize, nullptr, &pipeov); - - while (true) { - if (!check_timeout(start_time)) { - Log.error("minitouch start timeout"); - CloseHandle(m_process_info.hProcess); - CloseHandle(m_process_info.hThread); - CloseHandle(pipe_parent_read); - CloseHandle(pipe_parent_write); - return nullptr; - } - DWORD len = 0; - if (GetOverlappedResult(pipe_parent_read, &pipeov, &len, FALSE)) { - break; - } - } - return std::make_shared(pipe_parent_read, pipe_parent_write, m_process_info); } @@ -382,12 +356,32 @@ asst::IOHandlerWin32::~IOHandlerWin32() } } -std::string asst::IOHandlerWin32::read() +std::string asst::IOHandlerWin32::read(unsigned timeout_sec) { + auto check_timeout = [&](const auto& start_time) -> bool { + using namespace std::chrono_literals; + return std::chrono::steady_clock::now() - start_time < timeout_sec * 1s; + }; + + auto start_time = std::chrono::steady_clock::now(); + auto pipe_buffer = std::make_unique(PipeBufferSize); OVERLAPPED pipeov { .hEvent = CreateEventW(nullptr, TRUE, FALSE, nullptr) }; std::ignore = ReadFile(m_read, pipe_buffer.get(), PipeBufferSize, nullptr, &pipeov); - return std::string(pipe_buffer.get()); + + while (true) { + if (!check_timeout(start_time)) { + CancelIoEx(m_read, &pipeov); + Log.error("read timeout"); + break; + } + DWORD len = 0; + if (GetOverlappedResult(m_read, &pipeov, &len, FALSE)) { + break; + } + } + + return pipe_buffer.get(); } bool asst::IOHandlerWin32::write(const std::string_view data) diff --git a/src/MaaCore/Controller/Platform/Win32IO.h b/src/MaaCore/Controller/Platform/Win32IO.h index 7c208b2908..ff59cfb65e 100644 --- a/src/MaaCore/Controller/Platform/Win32IO.h +++ b/src/MaaCore/Controller/Platform/Win32IO.h @@ -69,7 +69,7 @@ namespace asst virtual ~IOHandlerWin32(); virtual bool write(const std::string_view data) override; - virtual std::string read() override; + virtual std::string read(unsigned timeout_sec) override; private: HANDLE m_read = INVALID_HANDLE_VALUE;