From 69a00825395774b12fc093dde0ffca25c3beee39 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 02:23:45 +0000 Subject: [PATCH 1/2] Let a second concurrent first save replace the first one's state file 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 ktsu-dev/Navigation#106 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW21vjGVn4pw5oZhxSF --- .gitignore | 18 +++++++++ .../JsonFilePersistenceProviderTests.cs | 36 ++++++++++++++++++ .../Services/JsonFilePersistenceProvider.cs | 37 +++++++++++++++---- 3 files changed, 83 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index dc0470a..e043c9f 100644 --- a/.gitignore +++ b/.gitignore @@ -203,6 +203,11 @@ PublishScripts/ **/[Pp]ackages/* # except build/, which is used as an MSBuild target. !**/[Pp]ackages/build/ +# and except a Unity project's Packages/, which is source: Unity's package manifest and its +# resolved lock file are both meant to be committed, and a NuGet restore folder never contains +# a file by either name. +!**/[Pp]ackages/manifest.json +!**/[Pp]ackages/packages-lock.json # Uncomment if necessary however generally it will be regenerated when needed #!**/[Pp]ackages/repositories.config # NuGet v3's project.json files produces more ignorable files @@ -651,3 +656,16 @@ Temporary Items # ImGui.ini files imgui.ini + +# Game engine projects +# +# Godot: the import cache, and the mono/temp bin+obj a C# build writes. +.godot/ + +# Unity: .meta files are source, not the Visual Studio C++ build artifact that the `*.meta` rule +# further up targets. Unity generates one per asset and it carries the GUID that scenes, prefabs +# and serialized references point at, so ignoring them gives every clone fresh GUIDs and silently +# breaks those references - including for a plug-in whose .dll is itself a build output. This +# negation has to come after that rule to win, and is scoped to the asset tree so the Visual +# Studio artifact stays ignored everywhere else. +!**/[Aa]ssets/**/*.meta diff --git a/Navigation.Test/JsonFilePersistenceProviderTests.cs b/Navigation.Test/JsonFilePersistenceProviderTests.cs index 8f21969..ff2a58e 100644 --- a/Navigation.Test/JsonFilePersistenceProviderTests.cs +++ b/Navigation.Test/JsonFilePersistenceProviderTests.cs @@ -56,6 +56,42 @@ public async Task SaveThenLoad_IntoNewNavigation_RestoresCurrentAndStacks() Assert.AreEqual("c", string.Join(",", restored.GetForwardStack().Select(item => item.Id))); } + [TestMethod] + public async Task ConcurrentFirstSaves_ToNewFile_BothSucceed() + { + // Two saves that both find no state file used to race on File.Move, and the loser threw + // "already exists". Loop, because the window is narrow + string directory = Path.Combine(Path.GetTempPath(), $"navigation-{Guid.NewGuid():N}"); + try + { + for (int i = 0; i < 200; i++) + { + string filePath = Path.Combine(directory, $"state-{i}.json"); + JsonFilePersistenceProvider provider = new(filePath); + Navigation first = new(null, provider); + first.NavigateTo(new NavigationItem("a", "A")); + Navigation second = new(null, provider); + second.NavigateTo(new NavigationItem("b", "B")); + + await Task.WhenAll( + Task.Run(() => first.SaveStateAsync()), + Task.Run(() => second.SaveStateAsync())).ConfigureAwait(false); + + Navigation restored = new(null, provider); + Assert.IsTrue(await restored.LoadStateAsync().ConfigureAwait(false), $"iteration {i}"); + Assert.IsTrue(restored.Current?.Id is "a" or "b", $"iteration {i}"); + Assert.IsEmpty(Directory.GetFiles(directory, $"state-{i}.json.*.tmp"), $"iteration {i}"); + } + } + finally + { + if (Directory.Exists(directory)) + { + Directory.Delete(directory, recursive: true); + } + } + } + private string[] GetTemporaryFiles() => Directory.GetFiles(Path.GetDirectoryName(_filePath!)!, $"{Path.GetFileName(_filePath!)}.*.tmp"); diff --git a/Navigation/Services/JsonFilePersistenceProvider.cs b/Navigation/Services/JsonFilePersistenceProvider.cs index 61bb7e4..48f5835 100644 --- a/Navigation/Services/JsonFilePersistenceProvider.cs +++ b/Navigation/Services/JsonFilePersistenceProvider.cs @@ -74,14 +74,7 @@ public async Task SaveStateAsync(INavigationState state, CancellationToken ca cancellationToken.ThrowIfCancellationRequested(); - if (File.Exists(_filePath)) - { - File.Replace(tempPath, _filePath, destinationBackupFileName: null); - } - else - { - File.Move(tempPath, _filePath); - } + SwapIntoPlace(tempPath, _filePath); } catch { @@ -92,6 +85,34 @@ public async Task SaveStateAsync(INavigationState state, CancellationToken ca private static readonly UTF8Encoding Utf8NoBom = new(encoderShouldEmitUTF8Identifier: false); + /// + /// Moves a complete temporary file over the state file, whether or not the state file exists yet. + /// Two saves can both find no state file. Whichever moves second must replace the first one's file + /// rather than fail with "already exists", so the last writer wins as it does once the file exists. + /// + private static void SwapIntoPlace(string tempPath, string filePath) + { + if (File.Exists(filePath)) + { + File.Replace(tempPath, filePath, destinationBackupFileName: null); + return; + } + +#if NETCOREAPP3_0_OR_GREATER + File.Move(tempPath, filePath, overwrite: true); +#else + try + { + File.Move(tempPath, filePath); + } + catch (IOException) when (File.Exists(filePath)) + { + // Another save created the file after the check above + File.Replace(tempPath, filePath, destinationBackupFileName: null); + } +#endif + } + private static void TryDelete(string path) { try From 03603c294299fd90015018eb7bd5bb5c0deca946 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 02:31:28 +0000 Subject: [PATCH 2/2] Leave .gitignore as it is on main The SDK's build rewrote it, and the change is not part of this fix. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW21vjGVn4pw5oZhxSF --- .gitignore | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/.gitignore b/.gitignore index e043c9f..dc0470a 100644 --- a/.gitignore +++ b/.gitignore @@ -203,11 +203,6 @@ PublishScripts/ **/[Pp]ackages/* # except build/, which is used as an MSBuild target. !**/[Pp]ackages/build/ -# and except a Unity project's Packages/, which is source: Unity's package manifest and its -# resolved lock file are both meant to be committed, and a NuGet restore folder never contains -# a file by either name. -!**/[Pp]ackages/manifest.json -!**/[Pp]ackages/packages-lock.json # Uncomment if necessary however generally it will be regenerated when needed #!**/[Pp]ackages/repositories.config # NuGet v3's project.json files produces more ignorable files @@ -656,16 +651,3 @@ Temporary Items # ImGui.ini files imgui.ini - -# Game engine projects -# -# Godot: the import cache, and the mono/temp bin+obj a C# build writes. -.godot/ - -# Unity: .meta files are source, not the Visual Studio C++ build artifact that the `*.meta` rule -# further up targets. Unity generates one per asset and it carries the GUID that scenes, prefabs -# and serialized references point at, so ignoring them gives every clone fresh GUIDs and silently -# breaks those references - including for a plug-in whose .dll is itself a build output. This -# negation has to come after that rule to win, and is scoped to the asset tree so the Visual -# Studio artifact stays ignored everywhere else. -!**/[Aa]ssets/**/*.meta