Skip to content

state.json writes are not crash-safe: a torn write silently becomes an empty workspace #88

Description

@AThraen

Found during the v0.6.0 pre-release review. Not a regression — this behaviour predates the release — but it's the most likely way a user loses their whole workspace. Targeting v0.6.1.

The failure chain

StateService.SaveAsync does a bare overwrite, with no temp-file-then-rename and no backup:

await File.WriteAllTextAsync(path, json);

StateService.LoadAsync catches everything and returns a fresh empty state:

catch { return new AppState(); }

There are 33 SaveStateAsync() call sites — layout changes, session open/close/sleep, group edits — so the file is rewritten constantly. That gives:

  1. A write is interrupted (crash, power loss, forced kill during the ~29KB write) → truncated JSON on disk.
  2. Next launch loads it, throws, and silently returns an empty AppState. The user sees zero sessions and no error.
  3. Any of the 33 save triggers then fires and overwrites the truncated-but-forensically-recoverable file with empty state.

Step 2 is verified: truncated JSON, an empty file, and valid-JSON-of-the-wrong-shape all return a clean empty AppState with no signal to the user or to crash.log.

For context, a real state file here is ~29KB with 40 sessions, 5 groups and 7 group layouts. That's the whole workspace.

Suggested fix

  • Atomic write: serialize to state.json.tmp, then File.Replace(tmp, path, path + ".bak"). That makes the swap atomic and leaves a one-generation backup for free.
  • Fall back on load failure: if state.json fails to parse, try state.json.bak before giving up.
  • Stop failing silently: log the parse failure to crash.log, and surface something in the UI. Starting empty is a reasonable last resort, but it should never be indistinguishable from a genuine first run.
  • Consider not overwriting at all when the load failed and the recovery path also failed — better to refuse to save than to destroy the evidence.

Related, smaller

"Sessions": null (as opposed to the key being absent) deserializes to a genuine null list and NREs on first use — property initializers only apply when the key is missing, not when it's explicitly null. The app never writes null, so the realistic trigger is ImportExportService reading a hand-made file. A null-coalesce in LoadAsync covers it:

state.Sessions ??= [];
state.Groups ??= [];
state.RecentlyClosed ??= [];
state.GroupLayouts ??= new();

Not affected

Schema back-compat itself is fine and was verified against a real v0.5.0 file: 40 sessions, 5 groups and 11 run commands round-trip losslessly, new fields default correctly (ModeProcess, PostRunUrl → null, RecentlyClosed[]), and downgrading is safe (unknown properties are skipped, an unknown LastLayout falls back to Single). This issue is purely about durability of the write, not the shape of the data.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions