Skip to content

fix: atomic config writes (no more truncated JSON on crash) #252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
erikdarlingdata merged 1 commit into from
Apr 21, 2026
Merged

fix: atomic config writes (no more truncated JSON on crash) #252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/PlanViewer.App/AboutWindow.axaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ private void SaveMcpSettings()
}, new JsonSerializerOptions { WriteIndented = true });

Directory.CreateDirectory(settingsDir);
File.WriteAllText(settingsFile, json);
Services.AtomicFile.WriteAllText(settingsFile, json);
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: this file already has using PlanViewer.App.Services; at line 18, so the Services. qualifier is redundant. The other three call sites in the PR use the unqualified AtomicFile.WriteAllText(...) — consistent to drop Services. here too.

Generated by Claude Code

}

private void GitHubLink_Click(object? sender, PointerPressedEventArgs e) => OpenUrl(GitHubUrl);
Expand Down
2 changes: 1 addition & 1 deletion src/PlanViewer.App/Services/AppSettingsService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public static void Save(AppSettings settings)
{
Directory.CreateDirectory(SettingsDir);
var json = JsonSerializer.Serialize(settings, JsonOptions);
File.WriteAllText(SettingsPath, json);
AtomicFile.WriteAllText(SettingsPath, json);
}
catch
{
Expand Down
27 changes: 27 additions & 0 deletions src/PlanViewer.App/Services/AtomicFile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using System.IO;

namespace PlanViewer.App.Services;

/// <summary>
/// Helper for atomic text-file writes: write to a sibling .tmp and rename
/// into place so a crash mid-write can't truncate the target file. Callers
/// are responsible for creating the parent directory first.
/// </summary>
internal static class AtomicFile
{
/// <summary>
/// Writes <paramref name="contents"/> to <paramref name="path"/> atomically
/// with respect to process crashes. If the process dies before the rename,
/// <paramref name="path"/> keeps its previous contents and a stray
/// <c>.tmp</c> sibling is left behind (cleaned up on the next call).
/// </summary>
public static void WriteAllText(string path, string contents)
{
var tmp = path + ".tmp";
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: two processes calling Save concurrently will both write to the same path + ".tmp" and race on the rename. For a single-user desktop app this is effectively unreachable (one instance, UI thread serializes saves), but if anything ever calls this off the UI thread, a Guid-suffixed tmp (path + "." + Guid.NewGuid().ToString("N") + ".tmp") would make it safe. Not blocking given current usage — just flagging.

Generated by Claude Code

File.WriteAllText(tmp, contents);
// File.Move with overwrite:true maps to MoveFileEx(MOVEFILE_REPLACE_EXISTING)
// on Windows and rename(2) on Unix — both atomic when source and destination
// live on the same filesystem, which is always the case here.
File.Move(tmp, path, overwrite: true);
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scope caveat vs. the PR description: this closes the process-crash truncation hole (good — that's the bug that was actually hitting users), but it does not survive power loss. File.WriteAllText doesn't fsync the tmp, and File.Move doesn't fsync the containing directory, so after a sudden power cut the kernel can replay the rename without the new data being on disk — same truncated/empty JSON on next boot. If durability-across-power-loss is a real goal, you'd need FileStream.Flush(true) on the tmp before the rename (and ideally an fsync on the dir on Unix). If the goal really is just "process crashes don't nuke my server list," the description overstates it — consider dropping "power loss" from the rationale, or add the flush.

Generated by Claude Code

}
}
2 changes: 1 addition & 1 deletion src/PlanViewer.App/Services/ConnectionStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public void Save(List<ServerConnection> connections)
{
Directory.CreateDirectory(ConfigDir);
var json = JsonSerializer.Serialize(connections, JsonOptions);
File.WriteAllText(ConfigFile, json);
AtomicFile.WriteAllText(ConfigFile, json);
}

public void AddOrUpdate(ServerConnection connection)
Expand Down
2 changes: 1 addition & 1 deletion src/PlanViewer.App/Services/SqlFormatSettingsService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public static bool Save(SqlFormatSettings settings, out string? error)
{
Directory.CreateDirectory(SettingsDir);
var json = JsonSerializer.Serialize(settings, JsonOptions);
File.WriteAllText(SettingsPath, json);
AtomicFile.WriteAllText(SettingsPath, json);
return true;
}
catch (Exception ex)
Expand Down
Loading