From 8c4342853a5841e82c03c8b037ea9c8b9ee51344 Mon Sep 17 00:00:00 2001 From: Lion Kortlepel Date: Sun, 22 Sep 2024 18:52:50 +0200 Subject: [PATCH 1/5] refactor downloading The way it was done was so horrid, it was not only impossible to debug, with TODO comments saying it sucks, and other shit like that, but it was also just full of data races. You can rest easy however - I left most of the data races in there <3 For nostalgia (totally not because it's a massive pain to fix that). We now do single-threaded download, which can not only saturate my 100 Mbit/s line without any hickups, it can also go up to ~600000 Mbit/s for localhost transfers :) So I think it's fine. --- src/GameStart.cpp | 1 - src/Network/Resources.cpp | 117 +++++++++++++++++++++----------------- 2 files changed, 64 insertions(+), 54 deletions(-) diff --git a/src/GameStart.cpp b/src/GameStart.cpp index 6e49db9..241ad5e 100644 --- a/src/GameStart.cpp +++ b/src/GameStart.cpp @@ -64,7 +64,6 @@ std::string GetGamePath() { std::string Ver = CheckVer(GetGameDir()); Ver = Ver.substr(0, Ver.find('.', Ver.find('.') + 1)); Path += Ver + "/"; - info("Game user path: '" + Path + "'"); return Path; } #endif diff --git a/src/Network/Resources.cpp b/src/Network/Resources.cpp index 74ae789..2475243 100644 --- a/src/Network/Resources.cpp +++ b/src/Network/Resources.cpp @@ -7,6 +7,8 @@ /// #include "Network/network.hpp" +#include +#include #if defined(_WIN32) #include @@ -122,29 +124,41 @@ void AsyncUpdate(uint64_t& Rcv, uint64_t Size, const std::string& Name) { } while (!Terminate && Rcv < Size); } -char* TCPRcvRaw(SOCKET Sock, uint64_t& GRcv, uint64_t Size) { +std::vector TCPRcvRaw(SOCKET Sock, uint64_t& GRcv, uint64_t Size) { if (Sock == -1) { Terminate = true; UUl("Invalid Socket"); - return nullptr; + return {}; } - char* File = new char[Size]; + std::vector File(Size); uint64_t Rcv = 0; + + int i = 0; do { - int Len = int(Size - Rcv); - if (Len > 1000000) - Len = 1000000; + auto start = std::chrono::high_resolution_clock::now(); + + // receive at most some MB at a time + int Len = std::min(int(Size - Rcv), 2 * 1024 * 1024); int32_t Temp = recv(Sock, &File[Rcv], Len, MSG_WAITALL); if (Temp < 1) { info(std::to_string(Temp)); UUl("Socket Closed Code 1"); KillSocket(Sock); Terminate = true; - delete[] File; - return nullptr; + return {}; } Rcv += Temp; GRcv += Temp; + + // every 8th iteration calculate download speed for that iteration + if (i % 8 == 0) { + auto end = std::chrono::high_resolution_clock::now(); + auto difference = end - start; + float bits_per_s = float(Temp * 8) / float(std::chrono::duration_cast(difference).count()); + float megabits_per_s = bits_per_s / 1000; + debug("Download speed: " + std::to_string(uint32_t(megabits_per_s)) + "Mbit/s"); + } + ++i; } while (Rcv < Size && !Terminate); return File; } @@ -178,44 +192,34 @@ SOCKET InitDSock() { return DSock; } -std::string MultiDownload(SOCKET MSock, SOCKET DSock, uint64_t Size, const std::string& Name) { +std::vector MultiDownload(SOCKET MSock, SOCKET DSock, uint64_t Size, const std::string& Name) { + uint64_t GRcv = 0; - uint64_t GRcv = 0, MSize = Size / 2, DSize = Size - MSize; + uint64_t MSize = Size / 2; + uint64_t DSize = Size - MSize; - std::thread Au(AsyncUpdate, std::ref(GRcv), Size, Name); + std::thread Au([&] { AsyncUpdate(GRcv, Size, Name); }); - std::packaged_task task([&] { return TCPRcvRaw(MSock, GRcv, MSize); }); - std::future f1 = task.get_future(); - std::thread Dt(std::move(task)); - Dt.detach(); + const std::vector MData = TCPRcvRaw(MSock, GRcv, MSize); - char* DData = TCPRcvRaw(DSock, GRcv, DSize); - - if (!DData) { + if (MData.empty()) { MultiKill(MSock, DSock); - return ""; + return {}; } - f1.wait(); - char* MData = f1.get(); + const std::vector DData = TCPRcvRaw(DSock, GRcv, DSize); - if (!MData) { + if (DData.empty()) { MultiKill(MSock, DSock); - return ""; + return {}; } - if (Au.joinable()) - Au.join(); + Au.join(); - /// omg yes very ugly my god but i was in a rush will revisit - std::string Ret(Size, 0); - memcpy(&Ret[0], MData, MSize); - delete[] MData; - - memcpy(&Ret[MSize], DData, DSize); - delete[] DData; - - return Ret; + std::vector Result{}; + Result.insert(Result.begin(), MData.begin(), MData.end()); + Result.insert(Result.end(), DData.begin(), DData.end()); + return Result; } void InvalidResource(const std::string& File) { @@ -239,7 +243,7 @@ void SyncResources(SOCKET Sock) { Ret.clear(); int Amount = 0, Pos = 0; - std::string a, t; + std::string PathToSaveTo, t; for (const std::string& name : FNames) { if (!name.empty()) { t += name.substr(name.find_last_of('/') + 1) + ";"; @@ -267,21 +271,23 @@ void SyncResources(SOCKET Sock) { for (auto FN = FNames.begin(), FS = FSizes.begin(); FN != FNames.end() && !Terminate; ++FN, ++FS) { auto pos = FN->find_last_of('/'); if (pos != std::string::npos) { - a = "Resources" + FN->substr(pos); - } else + PathToSaveTo = "Resources" + FN->substr(pos); + } else { continue; + } Pos++; - if (fs::exists(a)) { + auto FileSize = std::stoull(*FS); + if (fs::exists(PathToSaveTo)) { if (FS->find_first_not_of("0123456789") != std::string::npos) continue; - if (fs::file_size(a) == std::stoull(*FS)) { - UpdateUl(false, std::to_string(Pos) + "/" + std::to_string(Amount) + ": " + a.substr(a.find_last_of('/'))); + if (fs::file_size(PathToSaveTo) == FileSize) { + UpdateUl(false, std::to_string(Pos) + "/" + std::to_string(Amount) + ": " + PathToSaveTo.substr(PathToSaveTo.find_last_of('/'))); std::this_thread::sleep_for(std::chrono::milliseconds(50)); try { if (!fs::exists(GetGamePath() + "mods/multiplayer")) { fs::create_directories(GetGamePath() + "mods/multiplayer"); } - auto modname = a.substr(a.find_last_of('/')); + auto modname = PathToSaveTo.substr(PathToSaveTo.find_last_of('/')); #if defined(__linux__) // Linux version of the game doesnt support uppercase letters in mod names for (char& c : modname) { @@ -290,7 +296,7 @@ void SyncResources(SOCKET Sock) { #endif auto name = GetGamePath() + "mods/multiplayer" + modname; auto tmp_name = name + ".tmp"; - fs::copy_file(a, tmp_name, fs::copy_options::overwrite_existing); + fs::copy_file(PathToSaveTo, tmp_name, fs::copy_options::overwrite_existing); fs::rename(tmp_name, name); } catch (std::exception& e) { error("Failed copy to the mods folder! " + std::string(e.what())); @@ -300,11 +306,12 @@ void SyncResources(SOCKET Sock) { WaitForConfirm(); continue; } else - remove(a.c_str()); + remove(PathToSaveTo.c_str()); } CheckForDir(); - std::string FName = a.substr(a.find_last_of('/')); + std::string FName = PathToSaveTo.substr(PathToSaveTo.find_last_of('/')); do { + debug("Loading file '" + FName + "' to '" + PathToSaveTo + "'"); TCPSend("f" + *FN, Sock); std::string Data = TCPRcv(Sock); @@ -316,19 +323,23 @@ void SyncResources(SOCKET Sock) { std::string Name = std::to_string(Pos) + "/" + std::to_string(Amount) + ": " + FName; - Data = MultiDownload(Sock, DSock, std::stoull(*FS), Name); + std::vector DownloadedFile = MultiDownload(Sock, DSock, FileSize, Name); if (Terminate) break; UpdateUl(false, std::to_string(Pos) + "/" + std::to_string(Amount) + ": " + FName); - std::ofstream LFS; - LFS.open(a.c_str(), std::ios_base::app | std::ios::binary); - if (LFS.is_open()) { - LFS.write(&Data[0], Data.size()); - LFS.close(); - } - } while (fs::file_size(a) != std::stoull(*FS) && !Terminate); + // 1. write downloaded file to disk + { + std::ofstream OutFile(PathToSaveTo, std::ios::binary | std::ios::trunc); + OutFile.write(DownloadedFile.data(), DownloadedFile.size()); + } + // 2. verify size + if (std::filesystem::file_size(PathToSaveTo) != DownloadedFile.size()) { + error("Failed to write the entire file '" + PathToSaveTo + "' correctly (file size mismatch)"); + Terminate = true; + } + } while (fs::file_size(PathToSaveTo) != std::stoull(*FS) && !Terminate); if (!Terminate) { if (!fs::exists(GetGamePath() + "mods/multiplayer")) { fs::create_directories(GetGamePath() + "mods/multiplayer"); @@ -341,7 +352,7 @@ void SyncResources(SOCKET Sock) { } #endif - fs::copy_file(a, GetGamePath() + "mods/multiplayer" + FName, fs::copy_options::overwrite_existing); + fs::copy_file(PathToSaveTo, GetGamePath() + "mods/multiplayer" + FName, fs::copy_options::overwrite_existing); } WaitForConfirm(); } From 191fbf083de4b11bd59b86289904e94d00afe2bf Mon Sep 17 00:00:00 2001 From: Lion Kortlepel Date: Sun, 22 Sep 2024 19:06:46 +0200 Subject: [PATCH 2/5] fix stupid microsoft macro <3 --- src/Network/Resources.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Network/Resources.cpp b/src/Network/Resources.cpp index 2475243..57fb8d9 100644 --- a/src/Network/Resources.cpp +++ b/src/Network/Resources.cpp @@ -124,6 +124,9 @@ void AsyncUpdate(uint64_t& Rcv, uint64_t Size, const std::string& Name) { } while (!Terminate && Rcv < Size); } +// MICROSOFT, I DONT CARE, WRITE BETTER CODE +#undef min + std::vector TCPRcvRaw(SOCKET Sock, uint64_t& GRcv, uint64_t Size) { if (Sock == -1) { Terminate = true; From a5766639d6f22e4154edb82fe15527bcad869846 Mon Sep 17 00:00:00 2001 From: Lion Kortlepel Date: Sun, 22 Sep 2024 19:29:39 +0200 Subject: [PATCH 3/5] add back user path print Thanks @WiserTixx for finding a good place for it --- src/Startup.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Startup.cpp b/src/Startup.cpp index dff8547..bfe9ccf 100644 --- a/src/Startup.cpp +++ b/src/Startup.cpp @@ -317,6 +317,7 @@ void PreGame(const std::string& GamePath) { info("Game Version : " + GameVer); CheckMP(GetGamePath() + "mods/multiplayer"); + info("Game user path: " + GetGamePath()); if (!Dev) { std::string LatestHash = HTTP::Get("https://backend.beammp.com/sha/mod?branch=" + Branch + "&pk=" + PublicKey); From 79209219dda97b3d344490737165ff8edeaae5a2 Mon Sep 17 00:00:00 2001 From: Lion Kortlepel Date: Sun, 22 Sep 2024 19:42:55 +0200 Subject: [PATCH 4/5] remove extraneous game user path print --- src/GameStart.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/GameStart.cpp b/src/GameStart.cpp index 241ad5e..ac99d63 100644 --- a/src/GameStart.cpp +++ b/src/GameStart.cpp @@ -51,7 +51,6 @@ std::string GetGamePath() { std::string Ver = CheckVer(GetGameDir()); Ver = Ver.substr(0, Ver.find('.', Ver.find('.') + 1)); Path += Ver + "\\"; - info("Game user path: '" + Path + "'"); return Path; } #elif defined(__linux__) From 96c9c8923861dfe33e21e4459b491a7672cb2c63 Mon Sep 17 00:00:00 2001 From: Lion Kortlepel Date: Sun, 22 Sep 2024 19:52:52 +0200 Subject: [PATCH 5/5] add extra layer of checks for data races in download yeah --- src/Network/Resources.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Network/Resources.cpp b/src/Network/Resources.cpp index 57fb8d9..aa47453 100644 --- a/src/Network/Resources.cpp +++ b/src/Network/Resources.cpp @@ -217,6 +217,13 @@ std::vector MultiDownload(SOCKET MSock, SOCKET DSock, uint64_t Size, const return {}; } + // ensure that GRcv is good before joining the async update thread + GRcv = MData.size() + DData.size(); + if (GRcv != Size) { + error("Something went wrong during download; didn't get enough data. Expected " + std::to_string(Size) + " bytes, got " + std::to_string(GRcv) + " bytes instead"); + return {}; + } + Au.join(); std::vector Result{};