From 6a5bf70be9d77bdd6dece38f3ec322e42b6e8768 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Sj=C3=B6lund?= Date: Wed, 26 Aug 2026 09:59:32 +0200 Subject: [PATCH] Write settings.json atomically `SetFlag` does a read-modify-write of `$HOME/.omsimulator/settings.json` for the two flags that carry `settings` (`--tempDir` and `--workingDir`). `SaveSettings` opened a `std::ofstream` directly on that path, which truncates it. Every OMSimulator process on the machine shares the file, so a concurrent run could parse it while it was empty or half written: ``` warning: Failed to save settings: [json.exception.parse_error.101] parse error at line 1, column 1: attempting to parse an empty input; check that your input string or stream contains the expected JSON ``` This shows up in the OpenModelica testsuite, where 11 `.mos` tests pass `--tempDir` to OMSimulator and rtest runs them in parallel under one `HOME`. 40 concurrent `--tempDir=... --version` runs produced 23 such warnings, one of them from a reader that caught a partially written file (`line 3, column 2: syntax error ... unexpected '}'`). Write to `settings.json..tmp` in the same directory and rename it into place instead, so a reader always sees one complete version of the file. `LoadSettings` now treats an empty file as "no settings" rather than throwing, which also covers a file left behind by a process killed mid-write. The same 40-way race is clean afterwards. Assisted-by: Claude Opus 5 (1M context) --- src/OMSimulatorLib/Flags.cpp | 89 +++++++++++++++++++++++------------- 1 file changed, 58 insertions(+), 31 deletions(-) diff --git a/src/OMSimulatorLib/Flags.cpp b/src/OMSimulatorLib/Flags.cpp index ac1f92046..43ce52833 100644 --- a/src/OMSimulatorLib/Flags.cpp +++ b/src/OMSimulatorLib/Flags.cpp @@ -46,6 +46,15 @@ #include #include #include +#include + +#if defined(_WIN32) || defined(_WIN64) +#include +#define OMS_GETPID _getpid +#else +#include +#define OMS_GETPID getpid +#endif oms::Flags::Flags() { @@ -70,54 +79,72 @@ oms::Flags &oms::Flags::GetInstance() return flags; } -void LoadSettings(nlohmann::json &json) +static std::filesystem::path SettingsDir() { #if defined(_WIN32) || defined(_WIN64) const char *homeDir = getenv("USERPROFILE"); #else const char *homeDir = getenv("HOME"); #endif - if (homeDir) - { - std::filesystem::path filePath = std::filesystem::path(homeDir) / ".omsimulator" / "settings.json"; - if (std::filesystem::exists(filePath)) - { - std::ifstream fileStream(filePath); - if (fileStream.is_open()) - { - json = nlohmann::json::parse(fileStream); - fileStream.close(); - } - } - } + return homeDir ? std::filesystem::path(homeDir) / ".omsimulator" : std::filesystem::path(); +} + +void LoadSettings(nlohmann::json &json) +{ + std::filesystem::path dir = SettingsDir(); + if (dir.empty()) + return; + + std::ifstream fileStream(dir / "settings.json"); + if (!fileStream.is_open()) + return; + + std::stringstream buffer; + buffer << fileStream.rdbuf(); + std::string content = buffer.str(); + + if (content.find_first_not_of(" \t\n\r") == std::string::npos) + return; + + json = nlohmann::json::parse(content); } void SaveSettings(nlohmann::json &json) { -#if defined(_WIN32) || defined(_WIN64) - const char *homeDir = getenv("USERPROFILE"); -#else - const char *homeDir = getenv("HOME"); -#endif - if (homeDir) + std::filesystem::path dir = SettingsDir(); + if (dir.empty()) { - std::filesystem::path path = std::filesystem::path(homeDir) / ".omsimulator"; - std::filesystem::path filePath = path / "settings.json"; - std::filesystem::create_directories(path); - std::ofstream fileStream(filePath); - if (fileStream.is_open()) + logWarning("HOME environment variable is not set, settings.json couldn't be saved"); + return; + } + + std::filesystem::create_directories(dir); + + std::filesystem::path tmpPath = dir / ("settings.json." + std::to_string(OMS_GETPID()) + ".tmp"); + { + std::ofstream fileStream(tmpPath, std::ios::binary | std::ios::trunc); + if (!fileStream.is_open()) { - fileStream << json.dump(2); - fileStream.close(); + logWarning("Failed to open settings.json for writing"); + return; } - else + fileStream << json.dump(2); + fileStream.flush(); + if (!fileStream.good()) { - logWarning("Failed to open settings.json for writing"); + fileStream.close(); + std::filesystem::remove(tmpPath); + logWarning("Failed to write settings.json"); + return; } } - else + + std::error_code ec; + std::filesystem::rename(tmpPath, dir / "settings.json", ec); + if (ec) { - logWarning("HOME environment variable is not set, settings.json couldn't be saved"); + std::filesystem::remove(tmpPath); + logWarning("Failed to update settings.json: " + ec.message()); } }