Skip to content

A crash mid-WriteText silently loses the newest save and leaves an orphaned .tmp file forever #310

Description

@matt-edmondson

What's wrong

AppData.WriteText<T> (AppDataStorage/AppData.cs:142-167) writes the new content to a .tmp file, then juggles the backup and original files before finally moving the temp file into place:

FileSystem.File.WriteAllText(tempFilePath, text);
try
{
    FileSystem.File.Delete(bkFilePath);
    FileSystem.File.Copy(appData.FilePath, bkFilePath);
    FileSystem.File.Delete(appData.FilePath);   // <- original deleted here
}
catch (FileNotFoundException) { }

FileSystem.File.Move(tempFilePath, appData.FilePath); // <- temp promoted here
FileSystem.File.Delete(bkFilePath);

If the process is killed (crash, power loss, OS kill) between the Delete(appData.FilePath) and the Move(tempFilePath, appData.FilePath), the resulting on-disk state is: main file missing, .tmp holding the newest data that was being saved, .bk holding the previous data.

AppData.ReadText<T> (lines 176-211) only knows how to recover from .bk on a FileNotFoundException — it never looks for a .tmp file. So on the next load, the newest save is silently discarded in favor of the stale backup, with no error surfaced to the caller, and the orphaned .tmp file is left on disk permanently (nothing ever reads or cleans it up again).

Verification

Reproduced directly: using MockFileSystem, set up on-disk state exactly as above (main file absent, .tmp = "Newest data that was being saved", .bk = "Original data") and called AppData.ReadText. Expected recovery of the newest data; actual result was "Original data" (the stale backup), with the .tmp file still present afterward.

Suggested fix

In ReadText's not-found handler, check for an orphaned .tmp file first — it represents the most recent write attempt — and promote it, falling back to .bk only when no .tmp exists. Additionally/alternately, restructure WriteText so the original file is never deleted before the replacement is fully in place (e.g. a single atomic rename of the original to backup, then move .tmp into the final name), and treat a leftover .tmp found at the start of WriteText/LoadOrCreate as evidence of an interrupted previous write that should be recovered rather than silently overwritten.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions