diff --git a/SerialPrograms/Source/CommonFramework/Startup/NewVersionCheck.cpp b/SerialPrograms/Source/CommonFramework/Startup/NewVersionCheck.cpp index bd4db94e50..704483c21c 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(3) ); }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..b7263143f2 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,16 @@ 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 +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); +JsonValue download_json_file( + Logger& logger, + const std::string& url, + std::chrono::seconds overall_timeout = std::chrono::seconds(0) +); }