Skip to content

Let a second concurrent first save replace the first one's state file [patch] - #108

Open
matt-edmondson wants to merge 2 commits into
mainfrom
fix/concurrent-first-save-106
Open

matt-edmondson wants to merge 2 commits into
mainfrom
fix/concurrent-first-save-106

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #106

What was wrong

JsonFilePersistenceProvider.SaveStateAsync swapped its temp file in with File.Move(temp, path) whenever the state file didn't exist yet. Two saves that both saw no file raced. The one that moved second threw IOException: ... already exists, its temp file was deleted, and the caller's save failed. That race only exists before the first save, so it hits an app's very first run.

Change

The swap moved into SwapIntoPlace:

  • The file exists: File.Replace, as before.
  • net8.0+ (NETCOREAPP3_0_OR_GREATER): File.Move(temp, path, overwrite: true), which is still an atomic rename.
  • netstandard2.0/2.1: File.Move. If that throws IOException and the file now exists, it falls back to File.Replace.

Either way the last writer wins, the same as when the file already exists. The atomicity from #105 doesn't change.

I left out the optional per-path SemaphoreSlim ordering. It changes behaviour beyond this bug and can be its own change.

Tests

  • New: ConcurrentFirstSaves_ToNewFile_BothSucceed. It runs 200 iterations of two concurrent SaveStateAsync calls against a path that doesn't exist yet. It checks that neither throws, the file loads back to one of the two states, and no temp file is left behind.
  • With the fix reverted, the test fails with the issue's IOException (JsonFilePersistenceProvider.cs:83).
  • Full suite on Linux / .NET 10: 66 total, 64 passed, 2 skipped (the existing Windows-only tests). The library builds for every target, including netstandard2.0/2.1.
  • The netstandard fallback compiles but isn't exercised, because the test project only targets net10.0.

Generated by Claude Code

Two saves that both found no state file raced on File.Move, and the loser
threw "already exists". The swap now overwrites on targets that support it,
and on netstandard falls back to File.Replace when the move loses the race.

Fixes #106

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW21vjGVn4pw5oZhxSF
The SDK's build rewrote it, and the change is not part of this fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW21vjGVn4pw5oZhxSF
@sonarqubecloud

sonarqubecloud Bot commented Nov 5, 2026

Copy link
Copy Markdown

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.

Two concurrent first saves to a new state file throw IOException "The file '…/state.json' already exists"

2 participants