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