From 51eb6b5c4d98e1b5c06b0e4854fab13eb69d1c41 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 19:33:50 +0000 Subject: [PATCH] Recover an interrupted write instead of the stale backup [patch] WriteText deletes the main file before moving the temp file into its place. A process killed in that window leaves the main file gone, the temp file holding the save that was in flight and the backup holding the content before it. ReadText only ever looked for the backup, so the next load silently restored the previous content and discarded the newest save, leaving the temp file orphaned on disk where nothing would ever read or clean it up. Try the temp file first and fall back to the backup, via a helper that carries the existing timestamped-archive behaviour for both. Archiving rather than deleting is what keeps LoadOrCreate's corrupt-file recovery terminating: it deletes a file it cannot deserialize and reads again, and a candidate left in place would be promoted again every time. Fixes #310 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Pup1E96GmvMiRTNMdABat --- AppDataStorage.Test/AppDataTests.cs | 48 +++++++++++++++++++++++++ AppDataStorage/AppData.cs | 55 +++++++++++++++++++++-------- 2 files changed, 88 insertions(+), 15 deletions(-) diff --git a/AppDataStorage.Test/AppDataTests.cs b/AppDataStorage.Test/AppDataTests.cs index b147f03..b63803e 100644 --- a/AppDataStorage.Test/AppDataTests.cs +++ b/AppDataStorage.Test/AppDataTests.cs @@ -260,6 +260,54 @@ public void TestReadTextRestoresFromBackupIfMainFileMissing() AssertFileExists(appData.FilePath, "Main file should be restored from backup."); } + // The on-disk state left by a process killed between WriteText's Delete of the main file and its + // Move of the temp file into place: main gone, temp holding the save that was in flight, backup + // holding the content before it. + private static void SetUpInterruptedWrite(TestAppData appData, string inFlight, string previous) + { + AppData.FileSystem.File.WriteAllText(AppData.MakeTempFilePath(appData.FilePath), inFlight); + AppData.FileSystem.File.WriteAllText(AppData.MakeBackupFilePath(appData.FilePath), previous); + AppData.FileSystem.File.Delete(appData.FilePath); + } + + [TestMethod] + public void TestReadTextRecoversTheInterruptedWriteRatherThanTheStaleBackup() + { + const string inFlight = "Newest data that was being saved"; + const string previous = "Original data"; + using TestAppData appData = CreateTestAppDataWithContent(TestDataString); + AppData.WriteText(appData, TestDataString); + SetUpInterruptedWrite(appData, inFlight, previous); + + string text = AppData.ReadText(appData); + + Assert.AreEqual(inFlight, text, "The newest save should be recovered, not the stale backup."); + AssertFileExists(appData.FilePath, "Main file should be restored from the interrupted write."); + } + + [TestMethod] + public void TestReadTextArchivesTheRecoveredTempFileRatherThanLeavingItOrphaned() + { + using TestAppData appData = CreateTestAppDataWithContent(TestDataString); + AppData.WriteText(appData, TestDataString); + SetUpInterruptedWrite(appData, "Newest data that was being saved", "Original data"); + AbsoluteFilePath tempFilePath = AppData.MakeTempFilePath(appData.FilePath); + + AppData.ReadText(appData); + + Assert.IsFalse( + AppData.FileSystem.File.Exists(tempFilePath), + "The consumed temp file should not be left orphaned on disk."); + + string directory = AppData.FileSystem.Path.GetDirectoryName(appData.FilePath)!; + string tempFileName = AppData.FileSystem.Path.GetFileName(tempFilePath); + Assert.IsTrue( + Array.Exists( + AppData.FileSystem.Directory.GetFiles(directory), + file => AppData.FileSystem.Path.GetFileName(file).StartsWith(tempFileName + ".", StringComparison.Ordinal)), + "The recovered temp file should be archived under a timestamped name, not deleted."); + } + [TestMethod] public void TestQueueSaveSetsSaveQueuedTime() { diff --git a/AppDataStorage/AppData.cs b/AppDataStorage/AppData.cs index 9eeaa5f..52290b3 100644 --- a/AppDataStorage/AppData.cs +++ b/AppDataStorage/AppData.cs @@ -186,22 +186,14 @@ internal static void EnsureDirectoryExists(AbsoluteDirectoryPath path) } catch (FileNotFoundException) { - AbsoluteFilePath bkFilePath = MakeBackupFilePath(appData.FilePath); - if (FileSystem.File.Exists(bkFilePath)) + // WriteText deletes the main file before moving the temp file into its place. A + // process killed in that window leaves the main file gone, the temp file holding the + // content of the save that was in flight, and the backup holding the content before + // it. The temp file is therefore the newer of the two candidates and is tried first; + // recovering the backup instead silently discards the newest save. + if (TryRestoreFrom(MakeTempFilePath(appData.FilePath), appData.FilePath) + || TryRestoreFrom(MakeBackupFilePath(appData.FilePath), appData.FilePath)) { - FileSystem.File.Copy(bkFilePath, appData.FilePath); - - // Create a unique timestamped backup filename - string timestamp = DateTime.Now.ToString("yyyyMMdd_HHmmss"); - AbsoluteFilePath timestampedBackup = bkFilePath.WithSuffix($".{timestamp}"); - int counter = 0; - while (FileSystem.File.Exists(timestampedBackup)) - { - counter++; - timestampedBackup = bkFilePath.WithSuffix($".{timestamp}_{counter}"); - } - - FileSystem.File.Move(bkFilePath, timestampedBackup); return ReadText(appData); } } @@ -210,6 +202,39 @@ internal static void EnsureDirectoryExists(AbsoluteDirectoryPath path) } } + /// + /// Restores from if the candidate exists, + /// then archives the candidate under a unique timestamped name. + /// + /// The recovery candidate to restore from. + /// The app data file path to restore. + /// True if the candidate existed and the file was restored; otherwise false. + private static bool TryRestoreFrom(AbsoluteFilePath candidate, AbsoluteFilePath filePath) + { + if (!FileSystem.File.Exists(candidate)) + { + return false; + } + + FileSystem.File.Copy(candidate, filePath); + + // Archiving rather than deleting keeps the recovered content for inspection, and moving it + // out of the way is what stops a candidate that turns out to be unreadable from being + // promoted again on the next attempt: LoadOrCreate deletes a file it cannot deserialize and + // reads again, which would otherwise restore the same bad content forever. + string timestamp = DateTime.Now.ToString("yyyyMMdd_HHmmss"); + AbsoluteFilePath archived = candidate.WithSuffix($".{timestamp}"); + int counter = 0; + while (FileSystem.File.Exists(archived)) + { + counter++; + archived = candidate.WithSuffix($".{timestamp}_{counter}"); + } + + FileSystem.File.Move(candidate, archived); + return true; + } + /// /// Queues a save operation for the current app data instance. ///