From 93601e4ecd660f3c8fdf6e9c3c00cf25a4103d7c Mon Sep 17 00:00:00 2001 From: Ivan Murashka Date: Tue, 6 Oct 2026 13:00:56 +0200 Subject: [PATCH] fix: keep the change when a storage write fails --- CHANGELOG.md | 1 + src/Runtime/BinaryStorage.cs | 14 +++- .../Persistence/BackgroundStorageWriter.cs | 20 ++++-- src/Runtime/Persistence/IStorageWriter.cs | 2 +- .../Persistence/ImmediateStorageWriter.cs | 3 +- src/Runtime/Persistence/StoragePersistence.cs | 4 +- src/Tests/Persistence/FailedWriteTests.cs | 68 +++++++++++++++++++ .../Persistence/FailedWriteTests.cs.meta | 2 + 8 files changed, 101 insertions(+), 13 deletions(-) create mode 100644 src/Tests/Persistence/FailedWriteTests.cs create mode 100644 src/Tests/Persistence/FailedWriteTests.cs.meta diff --git a/CHANGELOG.md b/CHANGELOG.md index e63dc39..ffcbca5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Two storages opened through paths that differ only in form (`a/save.dat` and `./a/save.dat`) no longer publish the same file without serializing against each other. - A storage larger than 1 MB no longer re-grows its serialization buffer from 1 MB on every save. - A save stopped between the two steps of `File.Replace` on Windows no longer loses the data: the next load reads the new file, or the backup if the new file cannot be read. +- A failed write no longer loses the change: the next change, `Save()` or `Dispose()` writes the data again. ## [1.0.5] - Baseline of the changelog. See the GitHub Releases page for earlier history. diff --git a/src/Runtime/BinaryStorage.cs b/src/Runtime/BinaryStorage.cs index a25690d..d57427f 100644 --- a/src/Runtime/BinaryStorage.cs +++ b/src/Runtime/BinaryStorage.cs @@ -663,11 +663,11 @@ public virtual void Dispose() return; } - if (AutoSave && _hasUnsavedChanges) + if (AutoSave && (_hasUnsavedChanges || !_persistence.Flush())) { SaveDataOnDisk(true); } - else + else if (!AutoSave) { _persistence.Flush(); } @@ -717,7 +717,15 @@ private void LoadDataFromDisk(KeyLoadFailedBehaviour keyLoadFailedBehaviour) private void SaveDataOnDisk(bool waitForDisk) { ThrowIfDisposed(); - _persistence.Save(_data, waitForDisk); + try + { + _persistence.Save(_data, waitForDisk); + } + catch + { + _hasUnsavedChanges = true; + throw; + } _hasUnsavedChanges = false; } diff --git a/src/Runtime/Persistence/BackgroundStorageWriter.cs b/src/Runtime/Persistence/BackgroundStorageWriter.cs index 832dec0..bcc9d51 100644 --- a/src/Runtime/Persistence/BackgroundStorageWriter.cs +++ b/src/Runtime/Persistence/BackgroundStorageWriter.cs @@ -19,6 +19,7 @@ internal sealed class BackgroundStorageWriter : IStorageWriter private StorageSnapshot? _pending; private bool _isScheduled; private bool _isPublishing; + private bool _lastWriteFailed; public BackgroundStorageWriter(StorageFile file) { @@ -32,17 +33,19 @@ public void Write(StorageSnapshot snapshot, bool waitForDisk) Schedule(snapshot); return; } - TakePending()?.Release(); + TakePending(out _)?.Release(); _file.Publish(snapshot); } - public void Flush() + public bool Flush() { - var pending = TakePending(); - if (pending != null) + var pending = TakePending(out var lastWriteFailed); + if (pending == null) { - _file.Publish(pending.Value); + return !lastWriteFailed; } + _file.Publish(pending.Value); + return true; } private void Schedule(StorageSnapshot snapshot) @@ -62,7 +65,7 @@ private void Schedule(StorageSnapshot snapshot) replaced?.Release(); } - private StorageSnapshot? TakePending() + private StorageSnapshot? TakePending(out bool lastWriteFailed) { lock (_lock) { @@ -70,6 +73,8 @@ private void Schedule(StorageSnapshot snapshot) { Monitor.Wait(_lock); } + lastWriteFailed = _lastWriteFailed; + _lastWriteFailed = false; var pending = _pending; _pending = null; return pending; @@ -91,18 +96,21 @@ private void PublishScheduled() _isPublishing = true; } + var failed = false; try { _file.Publish(snapshot); } catch (Exception exception) { + failed = true; Debug.LogError($"Failed to save storage '{_file.Main}'. Reason: {exception.Message}"); } finally { lock (_lock) { + _lastWriteFailed = failed; _isPublishing = false; Monitor.PulseAll(_lock); } diff --git a/src/Runtime/Persistence/IStorageWriter.cs b/src/Runtime/Persistence/IStorageWriter.cs index fef711a..70a4b01 100644 --- a/src/Runtime/Persistence/IStorageWriter.cs +++ b/src/Runtime/Persistence/IStorageWriter.cs @@ -4,6 +4,6 @@ internal interface IStorageWriter { void Write(StorageSnapshot snapshot, bool waitForDisk); - void Flush(); + bool Flush(); } } diff --git a/src/Runtime/Persistence/ImmediateStorageWriter.cs b/src/Runtime/Persistence/ImmediateStorageWriter.cs index c72a53b..232e1d4 100644 --- a/src/Runtime/Persistence/ImmediateStorageWriter.cs +++ b/src/Runtime/Persistence/ImmediateStorageWriter.cs @@ -14,8 +14,9 @@ public void Write(StorageSnapshot snapshot, bool waitForDisk) _file.Publish(snapshot); } - public void Flush() + public bool Flush() { + return true; } } } diff --git a/src/Runtime/Persistence/StoragePersistence.cs b/src/Runtime/Persistence/StoragePersistence.cs index e42de34..cce9df3 100644 --- a/src/Runtime/Persistence/StoragePersistence.cs +++ b/src/Runtime/Persistence/StoragePersistence.cs @@ -35,9 +35,9 @@ public void Save(Dictionary data, bool waitForDisk) } } - public void Flush() + public bool Flush() { - _writer.Flush(); + return _writer.Flush(); } private void SaveJsonCopy(IReadOnlyDictionary data) diff --git a/src/Tests/Persistence/FailedWriteTests.cs b/src/Tests/Persistence/FailedWriteTests.cs new file mode 100644 index 0000000..bd32e97 --- /dev/null +++ b/src/Tests/Persistence/FailedWriteTests.cs @@ -0,0 +1,68 @@ +using System; +using System.IO; +using System.Text.RegularExpressions; +using System.Threading; +using FluentAssertions; +using NUnit.Framework; +using UnityEngine; +using UnityEngine.TestTools; + +namespace Appegy.Storage +{ + public class FailedWriteTests : BaseStorageTests + { + [TearDown] + public void UnblockWrites() + { + if (Directory.Exists(TempPath)) + { + Directory.Delete(TempPath); + } + } + + [Test] + public void WhenBackgroundWriteFails_ThenDisposeWritesTheChange() + { + var storage = Open(autoSave: true); + Directory.CreateDirectory(TempPath); + using (var writeFailed = new ManualResetEventSlim()) + { + void OnLog(string message, string stackTrace, LogType type) + { + if (type == LogType.Error && message.StartsWith("Failed to save storage")) + { + writeFailed.Set(); + } + } + + LogAssert.Expect(LogType.Error, new Regex("Failed to save storage")); + Application.logMessageReceivedThreaded += OnLog; + storage.Set("value", 7); + var failed = writeFailed.Wait(TimeSpan.FromSeconds(10)); + Application.logMessageReceivedThreaded -= OnLog; + failed.Should().BeTrue(); + } + + Directory.Delete(TempPath); + storage.Dispose(); + + using var reopened = Open(); + reopened.Get("value").Should().Be(7); + } + + [Test] + public void WhenAutoSaveWriteFails_ThenDisposeWritesTheChange() + { + var storage = Open(autoSave: true, saveOnBackgroundThread: false); + Directory.CreateDirectory(TempPath); + + storage.Invoking(s => s.Set("value", 7)).Should().Throw(); + + Directory.Delete(TempPath); + storage.Dispose(); + + using var reopened = Open(); + reopened.Get("value").Should().Be(7); + } + } +} diff --git a/src/Tests/Persistence/FailedWriteTests.cs.meta b/src/Tests/Persistence/FailedWriteTests.cs.meta new file mode 100644 index 0000000..449403f --- /dev/null +++ b/src/Tests/Persistence/FailedWriteTests.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 311ef20eb9464350820a5db4538ab2c2 \ No newline at end of file