From 4f010c140f28ac8ffae4e7fee5d586eb30402763 Mon Sep 17 00:00:00 2001 From: Gin890 <79114795+Gin890@users.noreply.github.com> Date: Thu, 15 Oct 2026 07:57:47 +0000 Subject: [PATCH 1/2] Version check: bound the UI-thread download with an overall timeout Clicking Settings (and program stop/startup, Main.cpp) calls check_new_version() on the UI thread, which downloads LatestVersion.json synchronously. QNetworkRequest::setTransferTimeout(5s) only limits data transfer, not the connection phase, and the overall watchdog in download_file() is disabled (#if 0), so a connection that stalls to raw.githubusercontent.com blocks QEventLoop::exec() - and the UI thread - indefinitely (the app appears to hang when Settings is clicked). Add an optional overall deadline to download_file()/download_json_file() (default: no limit, so resource downloads keep their current behavior) and give the version check a 10 second budget; on expiry the reply is aborted and the existing OperationFailedException path skips the check. --- .../Startup/NewVersionCheck.cpp | 7 ++++- .../CommonFramework/Tools/FileDownloader.cpp | 26 ++++++++++++++++--- .../CommonFramework/Tools/FileDownloader.h | 10 +++++-- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/SerialPrograms/Source/CommonFramework/Startup/NewVersionCheck.cpp b/SerialPrograms/Source/CommonFramework/Startup/NewVersionCheck.cpp index bd4db94e50..990c72b7b0 100644 --- a/SerialPrograms/Source/CommonFramework/Startup/NewVersionCheck.cpp +++ b/SerialPrograms/Source/CommonFramework/Startup/NewVersionCheck.cpp @@ -204,9 +204,14 @@ void check_new_version(Logger& logger){ logger.log("Checking for new version..."); JsonValue json; try{ + // This runs on the UI thread (panel switching / on_idle), so the + // request gets a hard deadline: setTransferTimeout() only covers + // data transfer, and a connection that stalls would otherwise + // freeze the UI until the OS gives up on the TCP attempt. json = FileDownloader::download_json_file( logger, - "https://raw.githubusercontent.com/PokemonAutomation/ComputerControl/master/LatestVersion.json" + "https://raw.githubusercontent.com/PokemonAutomation/ComputerControl/master/LatestVersion.json", + std::chrono::seconds(10) ); }catch (OperationFailedException&){ return; diff --git a/SerialPrograms/Source/CommonFramework/Tools/FileDownloader.cpp b/SerialPrograms/Source/CommonFramework/Tools/FileDownloader.cpp index 16d1835711..83224cfe17 100644 --- a/SerialPrograms/Source/CommonFramework/Tools/FileDownloader.cpp +++ b/SerialPrograms/Source/CommonFramework/Tools/FileDownloader.cpp @@ -27,7 +27,7 @@ namespace PokemonAutomation{ namespace FileDownloader{ -std::string download_file(Logger& logger, const std::string& url){ +std::string download_file(Logger& logger, const std::string& url, std::chrono::seconds overall_timeout){ // cout << "download_file()" << endl; QNetworkAccessManager network_access_manager; QByteArray downloaded_data; @@ -57,6 +57,19 @@ std::string download_file(Logger& logger, const std::string& url){ request.setTransferTimeout(std::chrono::seconds(5)); reply.reset(network_access_manager.get(request)); + // Deadline for the whole request, including the connection phase + // (setTransferTimeout() above only limits data transfer). Without + // this, a connection that stalls would block loop.exec() forever. + QTimer overall_timer; + if (overall_timeout.count() > 0){ + overall_timer.setSingleShot(true); + QObject::connect( + &overall_timer, &QTimer::timeout, + &loop, [&loop]{ loop.exit(); } + ); + overall_timer.start((int)overall_timeout.count()); + } + #if 0 QTimer timer; timer.singleShot( @@ -72,11 +85,18 @@ std::string download_file(Logger& logger, const std::string& url){ // cout << "loop.exec() - enter" << endl; loop.exec(); // cout << "loop.exec() - exit" << endl; + overall_timer.stop(); if (!reply){ std::string str = "QNetworkReply is null."; // logger.log(str, COLOR_RED); throw_and_log(logger, ErrorReportMode::NO_ERROR_REPORT, str); + }else if (overall_timeout.count() > 0 && reply->isRunning()){ + // The overall deadline elapsed while the request (typically the + // connection) was still in progress. + reply->abort(); + throw_and_log(logger, ErrorReportMode::NO_ERROR_REPORT, + "Timed out after " + std::to_string(overall_timeout.count()) + " seconds downloading " + url + "."); }else if (reply->error() == QNetworkReply::NoError){ // QString contents = QString::fromUtf8(reply->readAll()); // qDebug() << contents; @@ -179,8 +199,8 @@ void download_file_to_disk( reply->deleteLater(); } -JsonValue download_json_file(Logger& logger, const std::string& url){ - std::string downloaded_data = download_file(logger, url); +JsonValue download_json_file(Logger& logger, const std::string& url, std::chrono::seconds overall_timeout){ + std::string downloaded_data = download_file(logger, url, overall_timeout); return parse_json(downloaded_data); } diff --git a/SerialPrograms/Source/CommonFramework/Tools/FileDownloader.h b/SerialPrograms/Source/CommonFramework/Tools/FileDownloader.h index 9920acf71b..3f52389db0 100644 --- a/SerialPrograms/Source/CommonFramework/Tools/FileDownloader.h +++ b/SerialPrograms/Source/CommonFramework/Tools/FileDownloader.h @@ -8,6 +8,7 @@ #define PokemonAutomation_FileDownloader_H #include +#include #include #include "Common/Cpp/CancellableScope.h" @@ -18,7 +19,12 @@ namespace PokemonAutomation{ namespace FileDownloader{ // Throws OperationFailedException if failed to download. -std::string download_file(Logger& logger, const std::string& url); +// overall_timeout is a deadline for the whole request including the +// connection phase (QNetworkRequest::setTransferTimeout() only limits +// data transfer, so without it a connection that stalls would block +// this call forever). std::chrono::seconds(0) (the default) imposes no +// overall limit. +std::string download_file(Logger& logger, const std::string& url, std::chrono::seconds overall_timeout = std::chrono::seconds(0)); // Throws OperationFailedException if failed to download. void download_file_to_disk( @@ -32,7 +38,7 @@ void download_file_to_disk( // Throws OperationFailedException if failed to download. // Returns empty value if invalid JSON. -JsonValue download_json_file(Logger& logger, const std::string& url); +JsonValue download_json_file(Logger& logger, const std::string& url, std::chrono::seconds overall_timeout = std::chrono::seconds(0)); } From e87b0d8c4fe15c2d5c59040b817125b8370c099d Mon Sep 17 00:00:00 2001 From: jw098 Date: Fri, 16 Oct 2026 12:28:25 -0700 Subject: [PATCH 2/2] minor changes --- .../CommonFramework/Startup/NewVersionCheck.cpp | 2 +- .../Source/CommonFramework/Tools/FileDownloader.h | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/SerialPrograms/Source/CommonFramework/Startup/NewVersionCheck.cpp b/SerialPrograms/Source/CommonFramework/Startup/NewVersionCheck.cpp index 990c72b7b0..704483c21c 100644 --- a/SerialPrograms/Source/CommonFramework/Startup/NewVersionCheck.cpp +++ b/SerialPrograms/Source/CommonFramework/Startup/NewVersionCheck.cpp @@ -211,7 +211,7 @@ void check_new_version(Logger& logger){ json = FileDownloader::download_json_file( logger, "https://raw.githubusercontent.com/PokemonAutomation/ComputerControl/master/LatestVersion.json", - std::chrono::seconds(10) + std::chrono::seconds(3) ); }catch (OperationFailedException&){ return; diff --git a/SerialPrograms/Source/CommonFramework/Tools/FileDownloader.h b/SerialPrograms/Source/CommonFramework/Tools/FileDownloader.h index 3f52389db0..b7263143f2 100644 --- a/SerialPrograms/Source/CommonFramework/Tools/FileDownloader.h +++ b/SerialPrograms/Source/CommonFramework/Tools/FileDownloader.h @@ -24,7 +24,11 @@ namespace FileDownloader{ // data transfer, so without it a connection that stalls would block // this call forever). std::chrono::seconds(0) (the default) imposes no // overall limit. -std::string download_file(Logger& logger, const std::string& url, std::chrono::seconds overall_timeout = std::chrono::seconds(0)); +std::string download_file( + Logger& logger, + const std::string& url, + std::chrono::seconds overall_timeout = std::chrono::seconds(0) +); // Throws OperationFailedException if failed to download. void download_file_to_disk( @@ -38,7 +42,11 @@ void download_file_to_disk( // Throws OperationFailedException if failed to download. // Returns empty value if invalid JSON. -JsonValue download_json_file(Logger& logger, const std::string& url, std::chrono::seconds overall_timeout = std::chrono::seconds(0)); +JsonValue download_json_file( + Logger& logger, + const std::string& url, + std::chrono::seconds overall_timeout = std::chrono::seconds(0) +); }