From 80242c0ec8114f430058daa30e2681f2b4fa29e2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Oct 2026 11:27:08 +0000 Subject: [PATCH] Cover deadline boundaries, safe cancellation and fresh join races Co-authored-by: Jean-LucTch <88255067+Jean-LucTch@users.noreply.github.com> --- CMakeLists.txt | 5 + .../Vfs/Test/https_download_timeout_tests.cpp | 334 ++++++++++++++++++ .../Test/map_download_timeout_tests.cpp | 205 ++++++++++- Extension/Assets/map_download.cpp | 56 ++- Extension/Multiplayer/Session/session.cpp | 10 +- Extension/Multiplayer/Session/session.h | 10 +- .../Multiplayer/Session/session_commands.cpp | 13 +- .../Multiplayer/Session/session_internal.h | 2 +- .../Multiplayer/Session/session_receive.cpp | 17 +- .../Test/map_fetch_policy_tests.cpp | 3 + Extension/Multiplayer/Test/mesh_tests.cpp | 79 +++++ Launcher/Test/mod_manager_tests.cpp | 22 ++ 12 files changed, 731 insertions(+), 25 deletions(-) create mode 100644 Engine/Vfs/Test/https_download_timeout_tests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 9dc13fea..d50bd1ca 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -66,6 +66,11 @@ if(DINGOSDK_BUILD_MULTIPLAYER_TESTS) target_include_directories(dingosdk_map_fetch_policy_tests PRIVATE "${PROJECT_SOURCE_DIR}") add_test(NAME map_fetch_policy COMMAND dingosdk_map_fetch_policy_tests) if(WIN32) + add_executable(dingosdk_https_download_timeout_tests Engine/Vfs/Test/https_download_timeout_tests.cpp) + target_include_directories(dingosdk_https_download_timeout_tests PRIVATE "${PROJECT_SOURCE_DIR}") + target_link_libraries(dingosdk_https_download_timeout_tests PRIVATE winhttp) + add_test(NAME https_download_timeout COMMAND dingosdk_https_download_timeout_tests + "${CMAKE_CURRENT_BINARY_DIR}/https-download-fixtures") add_executable(dingosdk_map_download_timeout_tests Extension/Assets/Test/map_download_timeout_tests.cpp Engine/Vfs/thunderstore_package.cpp) target_include_directories(dingosdk_map_download_timeout_tests PRIVATE "${PROJECT_SOURCE_DIR}") diff --git a/Engine/Vfs/Test/https_download_timeout_tests.cpp b/Engine/Vfs/Test/https_download_timeout_tests.cpp new file mode 100644 index 00000000..81acd506 --- /dev/null +++ b/Engine/Vfs/Test/https_download_timeout_tests.cpp @@ -0,0 +1,334 @@ +#include +#include +#include "../https_download.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace fake_http { +enum class Stage { none, send, receive, read, eof, close }; +struct Handle { + DWORD_PTR context{}; + WINHTTP_STATUS_CALLBACK callback{}; + DWORD notifications{}; + bool closed{}; +}; +struct State { + dingosdk::https::Watch* watch{}; + Stage stop_at{Stage::none}, fail_at{Stage::none}; + bool timeout{}, callback_error{}, content_length{true}, downgrade{}; + unsigned opens{}, reads{}, closing_callbacks{}, pending_buffer_closes{}; + DWORD redirect{}, request_flags{}, session_flags{}; + void* pending_buffer{}; + Handle session, connection, request; + std::string body{"payload"}; +}; +State state; + +void stop() { + if (state.timeout) state.watch->deadline_us.store(1); + else state.watch->cancel.store(true); +} +void notify(DWORD status, void* info = nullptr, DWORD size = 0) { + state.request.callback(&state.request, state.request.context, status, info, size); +} +bool pending(Stage stage) { + if (state.stop_at != stage) return false; + stop(); + return true; +} +bool failure(Stage stage) { + if (state.fail_at != stage) return false; + if (state.callback_error) { + WINHTTP_ASYNC_RESULT error{}; + error.dwError = ERROR_WINHTTP_CONNECTION_ERROR; + notify(WINHTTP_CALLBACK_STATUS_REQUEST_ERROR, &error, sizeof(error)); + } else { + SetLastError(ERROR_WINHTTP_CONNECTION_ERROR); + } + return true; +} +HINTERNET WINAPI open(LPCWSTR, DWORD, LPCWSTR, LPCWSTR, DWORD flags) { + ++state.opens; + state.session_flags = flags; + return &state.session; +} +HINTERNET WINAPI connect(HINTERNET, LPCWSTR, INTERNET_PORT, DWORD) { + return &state.connection; +} +HINTERNET WINAPI open_request(HINTERNET, LPCWSTR, LPCWSTR, LPCWSTR, LPCWSTR, LPCWSTR*, DWORD flags) { + state.request_flags = flags; + return &state.request; +} +BOOL WINAPI set_timeouts(HINTERNET, int, int, int, int) { return TRUE; } +BOOL WINAPI set_option(HINTERNET handle, DWORD option, LPVOID value, DWORD) { + if (option == WINHTTP_OPTION_CONTEXT_VALUE) + static_cast(handle)->context = *static_cast(value); + if (option == WINHTTP_OPTION_REDIRECT_POLICY) state.redirect = *static_cast(value); + return TRUE; +} +WINHTTP_STATUS_CALLBACK WINAPI set_callback(HINTERNET handle, WINHTTP_STATUS_CALLBACK callback, + DWORD notifications, DWORD_PTR) { + auto& request = *static_cast(handle); + const auto previous = request.callback; + request.callback = callback; + request.notifications = notifications; + return previous; +} +BOOL WINAPI send(HINTERNET, LPCWSTR, DWORD, LPVOID, DWORD, DWORD, DWORD_PTR context) { + state.request.context = context; + if (pending(Stage::send)) return TRUE; + if (failure(Stage::send)) return state.callback_error ? TRUE : FALSE; + notify(WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE); + return TRUE; +} +BOOL WINAPI receive(HINTERNET, LPVOID) { + if (pending(Stage::receive)) return TRUE; + if (failure(Stage::receive)) return state.callback_error ? TRUE : FALSE; + if (state.downgrade && state.redirect == WINHTTP_OPTION_REDIRECT_POLICY_DISALLOW_HTTPS_TO_HTTP) { + SetLastError(ERROR_WINHTTP_REDIRECT_FAILED); + return FALSE; + } + notify(WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE); + return TRUE; +} +BOOL WINAPI query(HINTERNET, DWORD query, LPCWSTR, LPVOID value, LPDWORD size, LPDWORD) { + if (query == (WINHTTP_QUERY_STATUS_CODE | WINHTTP_QUERY_FLAG_NUMBER)) { + *static_cast(value) = 200; + } else if (query == (WINHTTP_QUERY_CONTENT_LENGTH | WINHTTP_QUERY_FLAG_NUMBER) && state.content_length) { + *static_cast(value) = static_cast(state.body.size()); + } else { + SetLastError(ERROR_WINHTTP_HEADER_NOT_FOUND); + return FALSE; + } + *size = sizeof(DWORD); + return TRUE; +} +BOOL WINAPI read(HINTERNET, LPVOID buffer, DWORD capacity, LPDWORD count) { + // The production async path must receive the count through its callback. + if (count || capacity < state.body.size()) { + SetLastError(ERROR_INVALID_PARAMETER); + return FALSE; + } + if (++state.reads == 1) { + std::memcpy(buffer, state.body.data(), state.body.size()); + notify(WINHTTP_CALLBACK_STATUS_READ_COMPLETE, buffer, static_cast(state.body.size())); + } else { + if (state.stop_at == Stage::read) { + state.pending_buffer = buffer; + stop(); + return TRUE; + } + if (failure(Stage::read)) return state.callback_error ? TRUE : FALSE; + if (state.stop_at == Stage::eof) stop(); + notify(WINHTTP_CALLBACK_STATUS_READ_COMPLETE, buffer, 0); + } + return TRUE; +} +BOOL WINAPI close(HINTERNET handle) { + auto& object = *static_cast(handle); + if (object.closed) { + SetLastError(ERROR_INVALID_HANDLE); + return FALSE; + } + object.closed = true; + if (handle == &state.request && object.callback) { + if (state.pending_buffer) { + // Pending I/O can still touch the caller's buffer during closure. + *static_cast(state.pending_buffer) = 'x'; + ++state.pending_buffer_closes; + state.pending_buffer = nullptr; + } + if (state.stop_at == Stage::close) stop(); + if (object.context && (object.notifications & WINHTTP_CALLBACK_FLAG_HANDLES)) { + ++state.closing_callbacks; + notify(WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING); + } + } + return TRUE; +} +} + +#define WinHttpOpen fake_http::open +#define WinHttpConnect fake_http::connect +#define WinHttpOpenRequest fake_http::open_request +#define WinHttpSetTimeouts fake_http::set_timeouts +#define WinHttpSetOption fake_http::set_option +#define WinHttpSetStatusCallback fake_http::set_callback +#define WinHttpSendRequest fake_http::send +#define WinHttpReceiveResponse fake_http::receive +#define WinHttpQueryHeaders fake_http::query +#define WinHttpReadData fake_http::read +#define WinHttpCloseHandle fake_http::close +#include "../https_download.cpp" +#undef WinHttpOpen +#undef WinHttpConnect +#undef WinHttpOpenRequest +#undef WinHttpSetTimeouts +#undef WinHttpSetOption +#undef WinHttpSetStatusCallback +#undef WinHttpSendRequest +#undef WinHttpReceiveResponse +#undef WinHttpQueryHeaders +#undef WinHttpReadData +#undef WinHttpCloseHandle + +namespace { +int failures{}; +void check(bool condition, const char* what) { + if (condition) return; + std::cerr << "FAIL: " << what << '\n'; + ++failures; +} +std::string contents(const std::filesystem::path& path) { + std::ifstream input(path, std::ios::binary); + return {std::istreambuf_iterator(input), std::istreambuf_iterator()}; +} +struct Fixtures { + std::filesystem::path root; + bool created; + explicit Fixtures(std::filesystem::path path) : root(std::move(path)), + created(std::filesystem::create_directory(root)) {} + ~Fixtures() { + if (created) { + std::error_code error; + std::filesystem::remove_all(root, error); + } + } +}; +void reset(dingosdk::https::Watch& watch) { + fake_http::state = {}; + fake_http::state.watch = &watch; + watch.cancel.store(false); + watch.deadline_us.store(0); + watch.received.store(0); + watch.total.store(0); +} +} + +int main(int argc, char** argv) { + using namespace dingosdk::https; + using fake_http::Stage; + Fixtures fixtures(argc > 1 ? std::filesystem::path(argv[1]) : std::filesystem::current_path() / + ("https-download-timeout-fixtures-" + std::to_string(GetCurrentProcessId()))); + if (!fixtures.created) { + std::cerr << "Fixture directory already exists; refusing to overwrite it\n"; + return 1; + } + const auto path = fixtures.root / "download.bin"; + Watch watch; + const auto get = [&](std::uint64_t limit = 1024, std::uint32_t seconds = 5) { + return dingosdk::https::get(L"https://example.invalid/file", path, limit, seconds, L"test", &watch); + }; + const auto removed = [&] { return !std::filesystem::exists(path); }; + const auto closed = [&] { + return fake_http::state.closing_callbacks == 1 && fake_http::state.request.closed && + fake_http::state.connection.closed && fake_http::state.session.closed; + }; + + reset(watch); + auto result = get(); + check(result.ok && result.http_status == 200 && result.error == 0, "actual GET succeeds"); + check(contents(path) == "payload" && watch.received.load() == 7 && watch.total.load() == 7, + "successful bytes and progress are retained"); + check(closed(), "successful request receives final closing callback"); + check(fake_http::state.session_flags & WINHTTP_FLAG_ASYNC, "session uses asynchronous WinHTTP"); + check(fake_http::state.request_flags & WINHTTP_FLAG_SECURE, "request stays HTTPS"); + check(fake_http::state.redirect == WINHTTP_OPTION_REDIRECT_POLICY_DISALLOW_HTTPS_TO_HTTP, + "HTTPS downgrade redirect policy is configured"); + std::filesystem::remove(path); + + for (const auto stage : {Stage::send, Stage::receive, Stage::read, Stage::eof, Stage::close}) { + for (const bool timeout : {false, true}) { + reset(watch); + fake_http::state.stop_at = stage; + fake_http::state.timeout = timeout; + result = get(); + check(!result.ok && result.error == (timeout ? ERROR_TIMEOUT : ERROR_CANCELLED), + "pending operation or final completion reports the authoritative stop reason"); + check(removed(), "stopped GET removes only its created output, including partial bytes"); + check(closed(), "stopped request completes handle-closing lifetime handshake"); + if (stage == Stage::read) + check(watch.received.load() == 7 && fake_http::state.pending_buffer_closes == 1, + "partial output is cleaned while pending read buffer survives close"); + } + } + + for (const auto stage : {Stage::send, Stage::receive, Stage::read}) { + for (const bool callback_error : {false, true}) { + reset(watch); + fake_http::state.fail_at = stage; + fake_http::state.callback_error = callback_error; + result = get(); + check(!result.ok && result.error == ERROR_WINHTTP_CONNECTION_ERROR, + "immediate and callback API errors are reported"); + check(removed() && closed(), "API errors clean up output and close registered request"); + } + } + + reset(watch); + watch.deadline_us.store(1); + result = get(); + check(!result.ok && result.error == ERROR_TIMEOUT && fake_http::state.opens == 0 && removed(), + "already expired external deadline prevents network and output creation"); + reset(watch); + watch.deadline_us.store(static_cast(std::chrono::duration_cast( + std::chrono::steady_clock::now().time_since_epoch()).count()) + 60000000); + result = get(1024, 0); + check(!result.ok && result.error == ERROR_TIMEOUT && fake_http::state.opens == 0, + "internal timeout wins over a later external steady-clock deadline"); + reset(watch); + watch.cancel.store(true); + watch.deadline_us.store(1); + result = get(); + check(!result.ok && result.error == ERROR_CANCELLED && fake_http::state.opens == 0, + "explicit cancellation wins when both stop conditions hold"); + + reset(watch); + { + std::ofstream output(path, std::ios::binary); + output << "existing destination"; + } + result = get(); + check(!result.ok && contents(path) == "existing destination", + "CREATE_NEW failure preserves a pre-existing destination"); + check(closed(), "existing-destination failure still closes the request"); + std::filesystem::remove(path); + + for (const bool content_length : {false, true}) { + reset(watch); + fake_http::state.content_length = content_length; + result = get(3); + check(!result.ok && result.error == ERROR_FILE_TOO_LARGE && removed() && closed(), + "announced and streamed oversized bodies remain rejected and cleaned up"); + } + reset(watch); + fake_http::state.downgrade = true; + result = get(); + check(!result.ok && result.error == ERROR_WINHTTP_REDIRECT_FAILED && removed() && closed(), + "HTTPS-to-HTTP redirects remain refused"); + reset(watch); + fake_http::state.body.clear(); + result = get(); + check(!result.ok && result.error == ERROR_BAD_LENGTH && removed() && closed(), + "empty body remains rejected and its created output is removed"); + + reset(watch); + fake_http::state.stop_at = Stage::send; + const auto text = get_text(L"https://example.invalid/metadata", 1024, 5, L"test", &result, &watch); + check(!text && !result.ok && result.error == ERROR_CANCELLED && closed(), + "get_text forwards its Watch into the actual asynchronous GET"); + + if (failures) return 1; + std::cout << "HTTPS deadline/cancellation tests passed\n"; + return 0; +} diff --git a/Extension/Assets/Test/map_download_timeout_tests.cpp b/Extension/Assets/Test/map_download_timeout_tests.cpp index a1e2392f..e20fbdff 100644 --- a/Extension/Assets/Test/map_download_timeout_tests.cpp +++ b/Extension/Assets/Test/map_download_timeout_tests.cpp @@ -25,9 +25,12 @@ namespace fs = std::filesystem; namespace policy = dingosdk::multiplayer::map_fetch_policy; fs::path root; std::atomic clock{1000000}; +constexpr auto use_download_deadline = static_cast(-1); +std::atomic active_deadline{use_download_deadline}; std::atomic downloading{}, installing{}, release_download{}, release_install{}, merging{}; std::atomic fetching{true}, held{}, observed_cancel{}, installed_cancel{}; std::atomic downloads{}, installs{}, applies{}, stops{}, rejoins{}; +std::atomic queued_stop_deadline{}; bool block_download{}, block_install{}, slow{}, transfer_error{}, fallback{}; dingosdk::MultiplayerModel session; std::mutex messages_mutex, gate_mutex; @@ -157,9 +160,23 @@ std::string status() { return "applied"; } namespace dingosdk::multiplayer { MultiplayerModel model() { return fixture::session; } bool fetching_map() noexcept { return fixture::fetching.load(); } -void hold_map_load(bool hold) noexcept { fixture::held.store(hold); } -bool queue_command(std::string_view action, std::string_view, std::string_view) { +std::uint64_t map_fetch_deadline() noexcept { + const auto active = fixture::active_deadline.load(); + return active == fixture::use_download_deadline ? map_download::deadline.load() : active; +} +void hold_map_load(bool hold, std::string_view package) noexcept { + fixture::held.store(hold); + if (hold && package != "Team-Map-1.0.0") std::abort(); +} +bool queue_command(std::string_view action, std::string_view argument, std::string_view password) { if (action == "stop") ++fixture::stops; + if (action == "stop-map-fetch") { + const auto expected = map_download::deadline.load(); + fixture::require(expected && argument == std::to_string(expected) && password.empty(), + "map stop carries the captured originating attempt deadline"); + fixture::queued_stop_deadline.store(expected); + ++fixture::stops; + } if (action == "rejoin") ++fixture::rejoins; return true; } @@ -195,6 +212,7 @@ void reset() { require(download::view().stage == Stage::idle, "previous lifecycle did not finish"); fs::remove_all(root / "Mods"); clock.store(1000000); + active_deadline.store(use_download_deadline); downloading.store(false); installing.store(false); release_download.store(false); @@ -208,6 +226,7 @@ void reset() { installs.store(0); applies.store(0); stops.store(0); + queued_stop_deadline.store(0); rejoins.store(0); block_download = block_install = slow = transfer_error = fallback = false; session = {}; @@ -246,10 +265,13 @@ void joined() { require(download::view().stage == Stage::idle, "joined session clears lifecycle"); require(!held.load() && stops.load() == 0 && rejoins.load() == 0, "normal join does not leave or rejoin"); } -void idle(const char* reason) { +void idle(const char* reason, unsigned expected_stops = 1) { require(download::view().stage == Stage::idle, reason); drain(); - require(stops.load() == 1, "failure leaves waiting session exactly once"); + require(stops.load() == expected_stops, "failure only stops its originating waiting session"); + if (expected_stops) + require(queued_stop_deadline.load() == download::deadline.load(), + "queued map stop identifies the originating attempt for dispatch-time ownership validation"); download::tick(); require(held.load() == (merging.load() && download::apply_started.load()), "failure holds map loading only while an uncancellable live merge drains"); @@ -263,6 +285,35 @@ void tests() { require(downloads.load() == 1 && installs.load() == 1, "success downloads and installs exactly once"); joined(); + reset(); + merging.store(true); + offer(); + download::answer(true); + wait([] { return download::view().stage == Stage::applying; }, "unrelated live merge blocked map offer"); + drain(); + download::tick(); + require(applies.load() == 0 && held.load(), "map apply waits for unrelated live merge without rejecting offer"); + merging.store(false); + download::tick(); + require(applies.load() == 1, "map apply starts after unrelated merge drains"); + joined(); + + reset(); + block_download = true; + offer(); + download::answer(true); + wait([] { return downloading.load(); }, "download did not start for duplicate confirmation"); + const auto first_transfer_deadline = download::transfer_until.load(); + clock.fetch_add(policy::second_us); + download::answer(true); + require(download::transfer_until.load() == first_transfer_deadline && downloads.load() == 1 && + download::workers.load() == 1, + "only winning confirmation claims a worker and publishes the transfer deadline"); + release_download.store(true); + gate.notify_all(); + start_apply(); + joined(); + reset(); block_download = true; slow = true; @@ -314,6 +365,17 @@ void tests() { idle("install timeout"); require(installed_cancel.load() && applies.load() == 0 && noticed("could not be installed"), "installer observes cancellation"); + reset(); + block_install = true; + offer(); + download::answer(true); + wait([] { return installing.load(); }, "install did not start for progress timeout"); + clock.store(download::deadline.load()); + drain(); + idle("installer progress deadline"); + require(installed_cancel.load() && applies.load() == 0, + "installer progress callback enforces session deadline without client tick"); + reset(); offer(); applying(); @@ -350,6 +412,15 @@ void tests() { idle("confirmation tick timeout"); require(downloads.load() == 0 && noticed("two minutes"), "tick expires unanswered prompt"); + reset(); + const auto short_session = clock.load() + policy::confirmation_us / 2; + offer(short_session); + require(download::ask_until.load() == short_session, "confirmation is capped by existing session deadline"); + clock.store(short_session); + download::answer(true); + idle("shorter session confirmation boundary"); + require(downloads.load() == 0 && noticed("timed out"), "confirmation never extends existing session budget"); + reset(); offer(clock.load() + policy::completion_reserve_us); download::answer(true); @@ -385,7 +456,7 @@ void tests() { offer(); applying(); require(downloads.load() == 2, "retry fetches same package again"); - // Retry guard deliberately queued another stop above. + // The original user cancellation stopped its waiting attempt. stops.store(0); joined(); @@ -429,6 +500,130 @@ void tests() { download::answer(false); idle("host map change"); + reset(); + block_download = true; + offer(); + download::answer(true); + wait([] { return downloading.load(); }, "download did not start for session replacement"); + active_deadline.store(0); + download::tick(); + idle("session identity changed", 0); + require(observed_cancel.load() && installs.load() == 0, + "download cancels when the active session no longer owns its deadline"); + + reset(); + offer(); + active_deadline.store(0); + fetching.store(false); + session.connected = true; + download::tick(); + require(download::view().stage == Stage::idle && !held.load() && downloads.load() == 0 && + stops.load() == 0 && rejoins.load() == 0, + "abandoned confirmation cancels without holding or stopping a fresh unrelated ordinary join"); + download::answer(true); + require(downloads.load() == 0, "late confirmation cannot start download for abandoned session"); + + reset(); + offer(); + const auto abandoned_deadline = download::deadline.load(); + clock.fetch_add(policy::second_us); + const auto fresh_deadline = clock.load() + policy::session_us; + active_deadline.store(fresh_deadline); + download::offer({"Team-Map-1.0.0", "Test map", true, false, "", fresh_deadline}); + wait([] { return download::asking(); }, "fresh same-package need was not retaken"); + drain(); + require(download::deadline.load() == fresh_deadline && fresh_deadline != abandoned_deadline && + download::transfer_until.load() == 0 && stops.load() == 0, + "fresh same-package attempt replaces stale confirmation without stopping its new session"); + applying(); + joined(); + + reset(); + block_download = true; + offer(); + download::answer(true); + wait([] { return downloading.load(); }, "old download did not start for queued replacement"); + clock.fetch_add(policy::second_us); + const auto queued_deadline = clock.load() + policy::session_us; + active_deadline.store(queued_deadline); + download::offer({"Team-Map-1.0.0", "Test map", true, false, "", queued_deadline}); + download::tick(); + require(download::view().stage == Stage::idle && download::watch.cancel.load() && + downloads.load() == 1 && stops.load() == 0 && download::pending && + download::pending->deadline_us == queued_deadline, + "fresh offer queues while cancelled worker drains without stopping replacement session"); + drain(); + require(observed_cancel.load() && installs.load() == 0 && !fs::exists(archive), + "old worker cleans partial archive before queued replacement can use its globals"); + block_download = false; + download::tick(); + wait([] { return download::asking(); }, "queued same-package need was not offered after drain"); + drain(); + require(download::deadline.load() == queued_deadline && download::transfer_until.load() == 0 && + !download::pending && stops.load() == 0, + "queued attempt keeps fresh deadline and resets previous transfer budget"); + applying(); + require(downloads.load() == 2, "queued fresh attempt starts exactly one new transfer"); + joined(); + + reset(); + block_download = true; + offer(); + download::answer(true); + wait([] { return downloading.load(); }, "old download did not start for stale pending need"); + clock.fetch_add(policy::second_us); + const auto superseded_deadline = clock.load() + policy::session_us; + active_deadline.store(superseded_deadline); + download::offer({"Team-Map-1.0.0", "Test map", true, false, "", superseded_deadline}); + active_deadline.store(0); + drain(); + download::tick(); + require(download::view().stage == Stage::idle && !download::pending && downloads.load() == 1 && + stops.load() == 0 && !held.load(), + "queued need is discarded when its replacement session also ends before worker drain"); + + reset(); + offer(); + applying(); + merging.store(false); + download::tick(); + clock.store(download::join_at.load() + policy::installed_load_us - 1); + download::tick(); + require(download::view().stage == Stage::joining, "installed map load retains its last microsecond"); + clock.fetch_add(1); + download::tick(); + idle("installed map load timeout"); + require(noticed("still cannot load"), "installed map reports load failure at 30 seconds"); + + reset(); + offer(); + applying(); + merging.store(false); + download::tick(); + fetching.store(false); + session.active = false; + clock.fetch_add(2500000); + download::tick(); + require(download::view().stage == Stage::idle && rejoins.load() == 1 && stops.load() == 0, + "successful install rejoins when the original session ended independently"); + + reset(); + offer(); + applying(); + clock.store(download::deadline.load() - 2); + merging.store(false); + download::tick(); + require(download::view().stage == Stage::joining, "near-deadline apply reaches joining"); + clock.fetch_add(1); + session.connected = true; + fetching.store(false); + active_deadline.store(0); + clock.fetch_add(2); + download::tick(); + require(download::view().stage == Stage::idle && !held.load() && stops.load() == 0 && + rejoins.load() == 0 && !noticed("timed out"), + "join completed within session budget retires successfully even when next tick is after deadline"); + reset(); download::tick(); require(!held.load() && downloads.load() == 0 && stops.load() == 0 && rejoins.load() == 0, diff --git a/Extension/Assets/map_download.cpp b/Extension/Assets/map_download.cpp index 757fc8a4..2d37f8b8 100644 --- a/Extension/Assets/map_download.cpp +++ b/Extension/Assets/map_download.cpp @@ -17,6 +17,7 @@ #include #include #include +#include namespace dingosdk::map_download { namespace { @@ -34,6 +35,7 @@ std::atomic stage{Stage::idle}; std::atomic workers{}; std::atomic deadline{}, ask_until{}, transfer_until{}; multiplayer::MapNeed need; // under mutex, while not idle +std::optional pending; // a fresh attempt waiting for cancelled work to drain package::Name wanted; // under mutex std::string version; // under mutex: the one to download std::string newest; // under mutex @@ -68,17 +70,21 @@ bool need_server() { } // The session that is waiting on the map is left: there is no map to wait for. -void leave_session() { - if (multiplayer::fetching_map()) multiplayer::queue_command("stop", "", ""); +void leave_session(std::uint64_t expected = 0) { + const auto owned = expected ? expected : multiplayer::map_fetch_deadline(); + if (multiplayer::fetching_map() && owned) + multiplayer::queue_command("stop-map-fetch", std::to_string(owned), ""); } // Back to idle with the reason said, as the plain "map not installed" notice is. void fail(std::string reason, bool retry = false) { std::string text; bool update{}; + std::uint64_t failed_deadline{}; { std::lock_guard lock(mutex); if (stage.exchange(Stage::idle) == Stage::idle) return; + failed_deadline = deadline.load(); watch.cancel.store(true); stop_install.store(true); apply_wanted.store(false); @@ -88,7 +94,7 @@ void fail(std::string reason, bool retry = false) { : ", which is not installed on this PC. ") + reason; if (!retry) last_failed = need.package; } - leave_session(); + if (multiplayer::map_fetch_deadline() == failed_deadline) leave_session(failed_deadline); logging::log(logging::Level::warning, logging::Channel::assets, "Map download: {}", text); overlay::notify(overlay::NoticeLevel::warning, update ? "Map version differs" : "Map not installed", std::move(text)); } @@ -239,6 +245,10 @@ void fetch() { ? "The download failed (Thunderstore answered " + std::to_string(result.http_status) + ")." : "The download failed (Windows error " + std::to_string(result.error) + ")."); } + if (policy::expired(transfer, multiplayer::now_us())) { + fs::remove(archive, error); + return fail("Map download timed out. Please try joining the server again.", true); + } if (!advance(Stage::downloading, Stage::installing)) { fs::remove(archive, error); return; @@ -308,14 +318,20 @@ void start(void (*work)()) { } // namespace void offer(const multiplayer::MapNeed& asked) { - std::lock_guard lock(mutex); + std::unique_lock lock(mutex); + if (stage.load() != Stage::idle && asked.deadline_us && asked.deadline_us != deadline.load()) { + lock.unlock(); + fail("Map download cancelled.", true); + lock.lock(); + } // A cancelled worker/atomic live merge must drain before its globals or files are reused. if (stage.load() == Stage::idle && (workers.load() || (apply_started.load() && live_mods::busy()))) { - leave_session(); + pending = asked; overlay::notify(overlay::NoticeLevel::warning, "Map download", - "The previous map operation is still finishing. Please try joining again shortly."); + "The previous map operation is still finishing. Waiting before starting this map."); return; } + pending.reset(); auto idle = Stage::idle; const auto name = package::parse_name(asked.package); // (Taken before anything is set up, so a second need while this one is in hand is dropped.) @@ -359,7 +375,12 @@ void offer(const multiplayer::MapNeed& asked) { apply_wanted.store(false); apply_started.store(false); pick_download.store(true); - start(check); + try { + start(check); + } catch (...) { + lock.unlock(); + fail("The map download could not be started. Please try joining again.", true); + } } void answer(bool download) noexcept { @@ -432,9 +453,28 @@ View view() { } void tick() { + std::optional next; + { + std::lock_guard lock(mutex); + if (pending && pending->deadline_us != multiplayer::map_fetch_deadline()) pending.reset(); + if (pending && stage.load() == Stage::idle && !workers.load() && + !(apply_started.load() && live_mods::busy())) + next = std::exchange(pending, std::nullopt); + } + if (next) offer(*next); auto now = stage.load(); if (now != Stage::idle) { const auto session = multiplayer::model(); + // Admission (or completed travel) retires the session deadline. Do not turn an + // already successful join into a timeout while waiting out the UI's settling delay. + if (now == Stage::joining && session.active && session.connected && + !multiplayer::fetching_map() && !multiplayer::map_fetch_deadline()) { + std::lock_guard lock(mutex); + if (stage.compare_exchange_strong(now, Stage::idle)) last_failed.clear(); + now = stage.load(); + } + if (session.active && now != Stage::joining && multiplayer::map_fetch_deadline() != deadline.load()) + fail("Map download cancelled.", true); if (!session.active && session.status.find("stopped waiting for the map download") != std::string::npos) fail("The server stopped waiting for the map download.", true); if (continuing() && now == Stage::asking && @@ -448,7 +488,7 @@ void tick() { // The session leaves the map alone until it has been applied (an update's level is one the // game already has, and would load from files that are being replaced). multiplayer::hold_map_load((now != Stage::idle && now != Stage::joining) || - workers.load() || (apply_started.load() && live_mods::busy())); + workers.load() || (apply_started.load() && live_mods::busy()), need_package()); if (now == Stage::applying) { if (apply_wanted.load()) { // (A merge of the player's own may be running: wait for it and start this one.) diff --git a/Extension/Multiplayer/Session/session.cpp b/Extension/Multiplayer/Session/session.cpp index d7e05889..1bfd20fd 100644 --- a/Extension/Multiplayer/Session/session.cpp +++ b/Extension/Multiplayer/Session/session.cpp @@ -156,6 +156,7 @@ void stop(Session &s, std::string reason) { s.map_package.clear(); s.fetching_since = 0; s.map_fetch_deadline = 0; + map_fetch_deadline_flag.store(0); fetching_map_flag.store(false, std::memory_order_relaxed); s.world = 1; s.travelling = false; @@ -730,7 +731,10 @@ std::string take_leave_notice() { return std::exchange(session().leave_notice, { std::optional take_map_need() { return std::exchange(session().map_need, std::nullopt); } void set_map_package_lookup(MapPackageLookup lookup) noexcept { map_package_lookup.store(lookup, std::memory_order_relaxed); } bool fetching_map() noexcept { return fetching_map_flag.load(std::memory_order_relaxed); } -void hold_map_load(bool hold) noexcept { map_load_held.store(hold, std::memory_order_relaxed); } +std::uint64_t map_fetch_deadline() noexcept { return map_fetch_deadline_flag.load(); } +void hold_map_load(bool hold, std::string_view package) noexcept { + map_load_held.store(hold && !package.empty() ? map_hash(package) : 0); +} MultiplayerChat chat() { auto &s = session(); std::lock_guard lock(s.mutex); @@ -770,6 +774,7 @@ bool observe_local_world(Session &s, const NativeFrame &local, std::string_view if (local.ready && !s.awaiting_map && s.host_world_ready) { s.travelling = false; s.map_fetch_deadline = 0; + map_fetch_deadline_flag.store(0); s.last_world_ready = 0; for (auto &peer : active_peers(s)) { peer.last_packet = now; @@ -837,7 +842,7 @@ bool prepare_join_map(Session &s, bool ready, std::string_view current, MapLoade // installed one's place), and loads when the two agree. const auto have = std::string(other_map_version(s.map_package, host_map_package(s.join_destination))); // (Nor while its files are being put in place and applied: hold_map_load.) - const bool held = map_load_held.load(std::memory_order_relaxed); + const bool held = !s.map_package.empty() && map_load_held.load() == map_hash(s.map_package); const auto result = have.empty() && !held ? loader(s.join_destination, s.map_load_submitted, detail) : MapLoadResult::missing; if (result == MapLoadResult::missing) { const auto name = s.map_label.empty() ? world_level_name(world_destination_asset(s.join_destination)) : s.map_label; @@ -851,6 +856,7 @@ bool prepare_join_map(Session &s, bool ready, std::string_view current, MapLoade if (!s.fetching_since) { s.fetching_since = now; if (!s.map_fetch_deadline) s.map_fetch_deadline = now + map_fetch_limit_us; + map_fetch_deadline_flag.store(s.map_fetch_deadline); fetching_map_flag.store(true, std::memory_order_relaxed); s.map_need = MapNeed{s.map_package, name, dedicated_host(s), s.travelling, have, s.map_fetch_deadline}; s.rejoin = Session::Rejoin{{s.host_id, s.secret}, s.password, s.map_fetch_deadline}; diff --git a/Extension/Multiplayer/Session/session.h b/Extension/Multiplayer/Session/session.h index 6f4d4a02..3bd7e63b 100644 --- a/Extension/Multiplayer/Session/session.h +++ b/Extension/Multiplayer/Session/session.h @@ -28,7 +28,8 @@ std::string take_leave_notice(); // why the session just ended over a missing ma // for download instead (Extension/Assets/map_download.h): the session does not end. It tells // the host the map is being fetched, which is given much longer than a load, and goes on // trying the map, so it loads as soon as it is installed. Once per map; client thread. -// The downloader ends the session (queue_command "stop") when the player says no or the map +// The downloader ends its own attempt (queue_command "stop-map-fetch", with its deadline) +// when the player says no or the map // cannot be had. Should the session end meanwhile for another reason, // queue_command("rejoin", "", "") goes back to it, with the password it was joined with. struct MapNeed { @@ -45,11 +46,12 @@ struct MapNeed { std::optional take_map_need(); // Whether a session is waiting on a map being fetched right now. Thread-safe. bool fetching_map() noexcept; -// Held while a map is being downloaded, installed or applied: the session does not load the -// host's map meanwhile, and waits as it does for one that is missing. A map that is being +std::uint64_t map_fetch_deadline() noexcept; +// Held for the named package while it is being downloaded, installed or applied: unrelated +// joins are not held. The session waits as it does for a missing map. A map that is being // replaced by another version is one the game already knows, so without this it would load as // soon as the new files are in place, before the mods are applied again. Thread-safe. -void hold_map_load(bool hold) noexcept; +void hold_map_load(bool hold, std::string_view package) noexcept; // How a host learns the package of the map it is on, to tell its guests: the level's asset in, // the package (or nothing) out. Called on the client thread, often; it should be quick. using MapPackageLookup = std::string (*)(std::string_view asset); diff --git a/Extension/Multiplayer/Session/session_commands.cpp b/Extension/Multiplayer/Session/session_commands.cpp index 9bd4fc7b..c16b45c2 100644 --- a/Extension/Multiplayer/Session/session_commands.cpp +++ b/Extension/Multiplayer/Session/session_commands.cpp @@ -622,7 +622,7 @@ bool own_mark_command(std::string_view action) { } // namespace bool queue_command(std::string_view action, std::string_view argument, std::string_view password) { if (launcher::offline_mode() && !own_mark_command(action)) return false; - if ((action != "host" && action != "host-config" && action != "join" && action != "rejoin" && action != "join-lobby" && action != "join-friend-lobby" && action != "stop" && + if ((action != "host" && action != "host-config" && action != "join" && action != "rejoin" && action != "join-lobby" && action != "join-friend-lobby" && action != "stop" && action != "stop-map-fetch" && action != "distances" && action != "object-placement" && action != "object-limit" && action != "kick" && action != "clear-objects" && action != "nametags" && action != "chat-visible" && action != "chat-filter" && action != "nametag-distance" && action != "nametag-dots" && action != "nametags-friends" && action != "player-distance" && action != "direct-connections" && action != "pose-dump" && action != "vote" && action != "voice-chat" && @@ -875,7 +875,15 @@ std::string command(std::string_view action, std::string_view argument, std::str if (action == "status") return s.status + " | Players " + std::to_string(player_count(s)) + "/" + std::to_string(s.capacity); - if (action == "stop") { + if (action == "stop-map-fetch") { + std::uint64_t expected{}; + const auto parsed = std::from_chars(argument.data(), argument.data() + argument.size(), expected); + // A worker's queued cancellation may execute after a fresh join command. + if (parsed.ec != std::errc{} || parsed.ptr != argument.data() + argument.size() || + !expected || expected != s.map_fetch_deadline) + return s.status; + } + if (action == "stop" || action == "stop-map-fetch") { stop(s, "Multiplayer stopped."); publish(s); return s.status; @@ -1116,6 +1124,7 @@ std::string command(std::string_view action, std::string_view argument, std::str s.awaiting_map = true; s.join_started = now_us(); s.map_fetch_deadline = rejoin ? rejoin->deadline_us : 0; + map_fetch_deadline_flag.store(s.map_fetch_deadline); s.status = "Connecting and checking the host's map..."; } set_lobby_park_mode(s.mode == Mode::host || s.mode == Mode::join, s.mode == Mode::join); diff --git a/Extension/Multiplayer/Session/session_internal.h b/Extension/Multiplayer/Session/session_internal.h index 5ed4b9cf..2d7a6701 100644 --- a/Extension/Multiplayer/Session/session_internal.h +++ b/Extension/Multiplayer/Session/session_internal.h @@ -475,7 +475,7 @@ std::string answer_server_poll(Session &s, std::size_t answer); inline std::atomic server_vote_open_flag{}; inline std::atomic map_package_lookup{}; inline std::atomic fetching_map_flag{}; -inline std::atomic map_load_held{}; +inline std::atomic map_fetch_deadline_flag{}, map_load_held{}; // A guest without the map is downloading it: the clock they are loading against starts over // each time they say so, for as long as map_fetch_limit_us from the first time. inline void guest_fetching(Peer &peer, std::uint64_t now) { diff --git a/Extension/Multiplayer/Session/session_receive.cpp b/Extension/Multiplayer/Session/session_receive.cpp index 139c0f07..8510a55e 100644 --- a/Extension/Multiplayer/Session/session_receive.cpp +++ b/Extension/Multiplayer/Session/session_receive.cpp @@ -594,6 +594,10 @@ void networking(Session &s, const NativeFrame &local, std::uint64_t now) { continue; } const auto &p = *decoded; + if (s.mode == Mode::join && map_fetch_policy::expired(s.map_fetch_deadline, now)) { + stop(s, "Map download timed out. Please try joining the server again."); + return; + } if (s.mode == Mode::host && link->fetching_since && (!link->handshaken || link->travel_since) && map_fetch_policy::host_expired(link->fetching_since, now)) { disconnect(s, message.peer, "The server stopped waiting for the map download."); @@ -627,6 +631,7 @@ void networking(Session &s, const NativeFrame &local, std::uint64_t now) { s.map_package.clear(); s.fetching_since = 0; s.map_fetch_deadline = fetch_deadline; + map_fetch_deadline_flag.store(fetch_deadline); fetching_map_flag.store(false, std::memory_order_relaxed); s.map = 0; s.map_load_submitted = false; @@ -646,8 +651,6 @@ void networking(Session &s, const NativeFrame &local, std::uint64_t now) { s.host_world_ready = p.world_ready; link->world_ready = p.world_ready; link->last_packet = now; - if (s.mode == Mode::join && message.peer == s.host_id && !s.awaiting_map) - s.map_fetch_deadline = 0; if (s.travelling) s.status = p.world_ready ? "Host map ready. Waiting for local loading..." : "The host is changing maps. Your session stays connected."; @@ -748,6 +751,7 @@ void networking(Session &s, const NativeFrame &local, std::uint64_t now) { s.map_package = p.map_package; s.world = p.world; s.join_map_authorized |= p.map_authorized; + link->last_packet = now; if (new_challenge) s.last_map_request = 0; if (!s.join_map_authorized) @@ -841,6 +845,10 @@ void networking(Session &s, const NativeFrame &local, std::uint64_t now) { link->world_ready = true; link->travel_since = 0; link->last_packet = now; + if (s.mode == Mode::join && message.peer == s.host_id && !s.awaiting_map) { + s.map_fetch_deadline = 0; + map_fetch_deadline_flag.store(0); + } if (joined) s.join_backoff.joined(message.peer); if (s.mode == Mode::host) { send_required(s, message.peer, encode_wire(packet(s, PacketKind::welcome, now))); @@ -1152,6 +1160,8 @@ void networking(Session &s, const NativeFrame &local, std::uint64_t now) { continue; } const std::uint64_t handshake_timeout = p.map_authorized ? 180000000 : 8000000; + const bool fetching_from_host = s.mode == Mode::join && p.member.id == s.host_id && + s.map_fetch_deadline && !map_fetch_policy::expired(s.map_fetch_deadline, now); if (p.handshaken && (s.travelling || p.travel_since)) { if (p.travel_since && now - p.travel_since > 180000000) { disconnect(s, p.member.id, "A player could not finish loading the new map."); @@ -1159,7 +1169,8 @@ void networking(Session &s, const NativeFrame &local, std::uint64_t now) { } continue; } - if ((!p.handshaken && p.connected_at && now - p.connected_at > handshake_timeout) || + if ((!p.handshaken && p.connected_at && + (fetching_from_host ? now - p.last_packet > 10000000 : now - p.connected_at > handshake_timeout)) || (p.handshaken && now - p.last_packet > 10000000)) { disconnect(s, p.member.id, "A player timed out while waiting for gameplay data."); if (s.mode == Mode::off) diff --git a/Extension/Multiplayer/Test/map_fetch_policy_tests.cpp b/Extension/Multiplayer/Test/map_fetch_policy_tests.cpp index 6f998eb4..a263f1d0 100644 --- a/Extension/Multiplayer/Test/map_fetch_policy_tests.cpp +++ b/Extension/Multiplayer/Test/map_fetch_policy_tests.cpp @@ -1,4 +1,5 @@ #include "Extension/Multiplayer/Net/map_fetch_policy.h" +#include "Extension/Multiplayer/Net/protocol.h" #include #include #include @@ -15,6 +16,8 @@ int main() { try { require(policy::second_us == 1000000, "microsecond units"); require(policy::session_us == 1800 * policy::second_us, "30 minute session"); + require(dingosdk::multiplayer::map_fetch_limit_us == policy::session_us, + "host protocol and local downloader share the session budget"); require(policy::transfer_us == 1620 * policy::second_us, "27 minute transfer"); require(policy::completion_reserve_us == 180 * policy::second_us, "3 minute completion reserve"); require(policy::confirmation_us == 120 * policy::second_us, "2 minute confirmation"); diff --git a/Extension/Multiplayer/Test/mesh_tests.cpp b/Extension/Multiplayer/Test/mesh_tests.cpp index 229c92cd..09b428aa 100644 --- a/Extension/Multiplayer/Test/mesh_tests.cpp +++ b/Extension/Multiplayer/Test/mesh_tests.cpp @@ -1460,6 +1460,85 @@ void map_checks() { pair.run(1); check(pair.guest.mode == Mode::off, "Host departure left a pending map join active"); } + { + map_package_lookup.store(+[](std::string_view) { return std::string("Team-Map-1.0.0"); }); + MapPair pair; + MapPair::missing = true; + pair.run(20); + const auto deadline = pair.guest.map_fetch_deadline; + check(deadline && pair.guest.map_need && pair.guest.map_need->deadline_us == deadline, + "Downloader did not receive the session's authoritative deadline"); + pair.now += 180000001; + auto request = packet(pair.guest, PacketKind::map_request, pair.now); + request.map = pair.host.map; + request.map_fetching = true; + send_required(pair.guest, pair.host.host_id, encode_wire(request)); + networking(pair.host, pair.host_frame, pair.now); + networking(pair.guest, NativeFrame{}, pair.now); + check(pair.guest.mode == Mode::join && pair.guest.map_fetch_deadline == deadline, + "An active download was disconnected by the ordinary host-link handshake timeout"); + check(!prepare_join_map(pair.guest, false, pair.current, MapPair::loader, deadline - 1) && + pair.guest.mode == Mode::join, + "Map fetch expired before its deadline"); + check(!prepare_join_map(pair.guest, false, pair.current, MapPair::loader, deadline) && + pair.guest.mode == Mode::off, + "Map fetch survived its exact deadline"); + map_package_lookup.store(nullptr); + } + { + map_package_lookup.store(+[](std::string_view) { return std::string("Team-Map-1.0.0"); }); + MapPair pair; + MapPair::missing = true; + pair.run(20); + const auto deadline = pair.guest.map_fetch_deadline; + MapPair::missing = false; + pair.current = pair.host.map_name; + const auto now = deadline - 1; + check(prepare_join_map(pair.guest, true, pair.current, MapPair::loader, now), + "Installed map could not finish loading within the deadline"); + pair.guest.map = map_hash(pair.current); + pair.guest.map_name = pair.current; + find_peer(pair.guest, pair.host.host_id)->last_packet = now; // ongoing metadata heartbeats + check(pair.guest.map_fetch_deadline == deadline, + "Native loading completion prematurely discarded the join deadline"); + networking(pair.guest, pair.host_frame, now); + networking(pair.host, pair.host_frame, now); + networking(pair.guest, pair.host_frame, now); + check(find_peer(pair.guest, pair.host.host_id)->handshaken && !pair.guest.map_fetch_deadline, + "Welcome did not retire a successfully completed download deadline"); + check(prepare_join_map(pair.guest, true, pair.current, MapPair::loader, deadline + 1), + "A successfully joined player was disconnected by a retired deadline"); + map_package_lookup.store(nullptr); + } + { + MapPair pair; + pair.run(20); + hold_map_load(true, "Team-Unrelated-1.0.0"); + check(!prepare_join_map(pair.guest, false, pair.current, MapPair::loader, pair.now + 500000) && + pair.guest.mode == Mode::join && pair.guest.map_load_submitted, + "An abandoned download blocked an unrelated ordinary map load"); + hold_map_load(false, ""); + } + { + auto &waiting = session(); + stop(waiting, "Map cancellation fixture"); + waiting.mode = Mode::join; + waiting.map_fetch_deadline = 100; + check(queue_command("stop-map-fetch", "100", ""), "Map cancellation command was not queued"); + std::unique_ptr cancellation; + { + std::lock_guard lock(waiting.request_mutex); + cancellation = std::move(waiting.requests.back()); + waiting.requests.pop_back(); + } + // A fresh join executes before the old worker's cancellation is dispatched. + waiting.map_fetch_deadline = 200; + command(cancellation->action, cancellation->argument, cancellation->password); + check(waiting.mode == Mode::join && waiting.map_fetch_deadline == 200, + "A queued cancellation stopped a fresh map-fetch attempt"); + command("stop-map-fetch", "200"); + check(waiting.mode == Mode::off, "An owned cancellation did not stop its pending join"); + } std::cout << "Automatic map joining: codec, same/different maps, password-before-load, slow loading, " "skater readiness, missing maps, cancellation, timeout and host departure passed.\n"; } diff --git a/Launcher/Test/mod_manager_tests.cpp b/Launcher/Test/mod_manager_tests.cpp index e16c901a..6070274e 100644 --- a/Launcher/Test/mod_manager_tests.cpp +++ b/Launcher/Test/mod_manager_tests.cpp @@ -119,6 +119,28 @@ int main() { make_zip(sources / L"v2.zip", {{"Cool Park/layout.toc", "toc v2"}}); check(install(mods, sources / L"v2.zip", true) == "Cool Park" && read(mods / L"Cool Park" / L"layout.toc") == "toc v2", "Replacing installs the new copy"); + { + std::atomic cancel{}; + unsigned callbacks{}; + make_zip(sources / L"cancelled.zip", {{"Cool Park/layout.toc", "must not replace"}}); + const auto issue = error_of([&] { + launcher_mods::install(mods, sources / L"cancelled.zip", true, + [&](float) { if (++callbacks == 2) cancel.store(true); }, cancel); + }); + check(issue == "Install cancelled." && callbacks == 2, + "Cancellation at the final safe installation boundary was ignored"); + check(read(mods / L"Cool Park" / L"layout.toc") == "toc v2" && + !fs::exists(mods / L".reskate-install"), + "Cancelled replacement damaged the existing mod or left staging files"); + write(sources / L"cancelled folder" / L"Cool Park" / L"layout.toc", "must not replace"); + cancel.store(false); + callbacks = 0; + check(error_of([&] { + launcher_mods::install(mods, sources / L"cancelled folder", true, + [&](float) { if (++callbacks == 2) cancel.store(true); }, cancel); + }) == "Install cancelled." && read(mods / L"Cool Park" / L"layout.toc") == "toc v2", + "Cancelled folder replacement did not preserve the installed mod"); + } // Hostile or unrelated archives. make_zip(sources / L"evil.zip", {{"../evil.txt", "x"}, {"layout.toc", "toc"}});