From 7f503c33d22738c8781e5efbb8170ea4d6b2d83e Mon Sep 17 00:00:00 2001 From: Malkierian Date: Sat, 14 Sep 2024 19:38:22 -0700 Subject: [PATCH] Concurrency Fix (#4318) * Removed all CVarLoad uses from code to prevent thread concurrency issues. * Add mutext locks to save and load functions to prevent concurrent operations between those two. --- soh/soh/Enhancements/debugconsole.cpp | 3 +-- soh/soh/Enhancements/randomizer/3drando/rando_main.cpp | 3 +-- soh/soh/Enhancements/randomizer/randomizer.cpp | 3 +-- soh/soh/SaveManager.cpp | 5 +++++ soh/soh/SaveManager.h | 1 + 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/soh/soh/Enhancements/debugconsole.cpp b/soh/soh/Enhancements/debugconsole.cpp index ad51f1e4b..f1f3ac76b 100644 --- a/soh/soh/Enhancements/debugconsole.cpp +++ b/soh/soh/Enhancements/debugconsole.cpp @@ -1590,6 +1590,5 @@ void DebugConsole_Init(void) { {"group_name", Ship::ArgumentType::TEXT, true}, }}); - CVarSave(); - CVarLoad(); + Ship::Context::GetInstance()->GetWindow()->GetGui()->SaveConsoleVariablesOnNextTick(); } diff --git a/soh/soh/Enhancements/randomizer/3drando/rando_main.cpp b/soh/soh/Enhancements/randomizer/3drando/rando_main.cpp index 6f342535b..9705d9a14 100644 --- a/soh/soh/Enhancements/randomizer/3drando/rando_main.cpp +++ b/soh/soh/Enhancements/randomizer/3drando/rando_main.cpp @@ -22,8 +22,7 @@ void RandoMain::GenerateRando(std::unordered_map cvarS std::string fileName = Ship::Context::GetPathRelativeToAppDirectory(GenerateRandomizer(cvarSettings, excludedLocations, enabledTricks, seedString).c_str()); CVarSetString(CVAR_GENERAL("SpoilerLog"), fileName.c_str()); - CVarSave(); - CVarLoad(); + Ship::Context::GetInstance()->GetWindow()->GetGui()->SaveConsoleVariablesOnNextTick(); CVarSetInteger(CVAR_GENERAL("NewSeedGenerated"), 1); } diff --git a/soh/soh/Enhancements/randomizer/randomizer.cpp b/soh/soh/Enhancements/randomizer/randomizer.cpp index 487348c9d..d2a6c5029 100644 --- a/soh/soh/Enhancements/randomizer/randomizer.cpp +++ b/soh/soh/Enhancements/randomizer/randomizer.cpp @@ -3053,8 +3053,7 @@ void GenerateRandomizerImgui(std::string seed = "") { RandoMain::GenerateRando(cvarSettings, excludedLocations, enabledTricks, seed); CVarSetInteger(CVAR_GENERAL("RandoGenerating"), 0); - CVarSave(); - CVarLoad(); + Ship::Context::GetInstance()->GetWindow()->GetGui()->SaveConsoleVariablesOnNextTick(); generated = 1; } diff --git a/soh/soh/SaveManager.cpp b/soh/soh/SaveManager.cpp index b6fd0cb07..a1ea20b8a 100644 --- a/soh/soh/SaveManager.cpp +++ b/soh/soh/SaveManager.cpp @@ -17,6 +17,7 @@ #include #include #include +#include extern "C" SaveContext gSaveContext; using namespace std::string_literals; @@ -909,6 +910,7 @@ int copy_file(const char* src, const char* dst) { // Threaded SaveFile takes copy of gSaveContext for local unmodified storage void SaveManager::SaveFileThreaded(int fileNum, SaveContext* saveContext, int sectionID) { + saveMtx.lock(); SPDLOG_INFO("Save File - fileNum: {}", fileNum); // Needed for first time save, hasn't changed in forever anyway saveBlock["version"] = 1; @@ -983,6 +985,7 @@ void SaveManager::SaveFileThreaded(int fileNum, SaveContext* saveContext, int se InitMeta(fileNum); GameInteractor::Instance->ExecuteHooks(fileNum); SPDLOG_INFO("Save File Finish - fileNum: {}", fileNum); + saveMtx.unlock(); } // SaveSection creates a copy of gSaveContext to prevent mid-save data modification, and passes its reference to SaveFileThreaded @@ -1025,6 +1028,7 @@ void SaveManager::SaveGlobal() { } void SaveManager::LoadFile(int fileNum) { + saveMtx.lock(); SPDLOG_INFO("Load File - fileNum: {}", fileNum); std::filesystem::path fileName = GetFileName(fileNum); assert(std::filesystem::exists(fileName)); @@ -1087,6 +1091,7 @@ void SaveManager::LoadFile(int fileNum) { SohGui::RegisterPopup("Error loading save file", "A problem occurred loading the save in slot " + std::to_string(fileNum + 1) + ".\nSave file corruption is suspected.\n" + "The file has been renamed to prevent further issues."); } + saveMtx.unlock(); } void SaveManager::ThreadPoolWait() { diff --git a/soh/soh/SaveManager.h b/soh/soh/SaveManager.h index 817fc6fbf..df5499277 100644 --- a/soh/soh/SaveManager.h +++ b/soh/soh/SaveManager.h @@ -184,6 +184,7 @@ class SaveManager { nlohmann::json* currentJsonContext = nullptr; nlohmann::json::iterator currentJsonArrayContext; std::shared_ptr smThreadPool; + std::mutex saveMtx; }; #else