Skip to content

Write settings.json atomically - #1636

Merged
sjoelund merged 1 commit into
OpenModelica:masterfrom
sjoelund:atomic-settings-json
Aug 26, 2026
Merged

Write settings.json atomically#1636
sjoelund merged 1 commit into
OpenModelica:masterfrom
sjoelund:atomic-settings-json

Conversation

@sjoelund

Copy link
Copy Markdown
Member

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.<pid>.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)

Related Issues

Purpose

Approach

`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.<pid>.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)
@sjoelund
sjoelund force-pushed the atomic-settings-json branch from 27b6f2e to 6a5bf70 Compare August 26, 2026 08:03
@sjoelund
sjoelund merged commit 62c758f into OpenModelica:master Aug 26, 2026
4 checks passed
@sjoelund
sjoelund deleted the atomic-settings-json branch August 26, 2026 09:16
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 13.51351% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 29.24%. Comparing base (8ce1f2d) to head (6a5bf70).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/OMSimulatorLib/Flags.cpp 13.51% 29 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1636      +/-   ##
==========================================
- Coverage   29.26%   29.24%   -0.02%     
==========================================
  Files          68       68              
  Lines       12942    12960      +18     
  Branches     8378     8389      +11     
==========================================
+ Hits         3787     3790       +3     
- Misses       7965     7979      +14     
- Partials     1190     1191       +1     
Flag Coverage Δ
linux 29.24% <13.51%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant