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.
HarbourMasters · Sep 15, 2024 · 7f503c3 · 7f503c3
1 parent b5037a0
commit 7f503c3
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 6 deletions.
diff --git 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
@@ -22,8 +22,7 @@ void RandoMain::GenerateRando(std::unordered_map<RandomizerSettingKey, u8> 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
@@ -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
@@ -17,6 +17,7 @@
 #include <fstream>
 #include <filesystem>
 #include <array>
+#include <mutex>
 
 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<GameInteractor::OnSaveFile>(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
@@ -184,6 +184,7 @@ class SaveManager {
     nlohmann::json* currentJsonContext = nullptr;
     nlohmann::json::iterator currentJsonArrayContext;
     std::shared_ptr<BS::thread_pool> smThreadPool;
+    std::mutex saveMtx;
 };
 
 #else