Skip to content

settings.json is written non-atomically and a corrupt file is indistinguishable from first run, destroying user settings #509

Description

@thcp

User-confirmed data loss. A real settings.json was observed shrinking from five keys to one, losing port and allow_network, with the good backup then overwritten by the reduced state.

The defect

app/core/settings.py:111:

_SETTINGS_PATH.write_text(json.dumps(_ensure()), encoding="utf-8")

write_text opens with "w": it truncates first, then writes. Every other persistence path in the codebase uses temp+rename -- including _mirror_settings() eleven lines below, whose own comment says "a torn write here would be restored verbatim into the user's next install", and registry.persist(). The primary settings file is the only unprotected writer.

How the data actually disappears

  1. _save() truncates settings.json; the process dies mid-write (app quit, SIGTERM from the parent watchdog, power loss). The file now exists and is invalid JSON.
  2. _load() (settings.py:79-83) catches bare Exception, logs a warning, returns {}. It cannot distinguish "no file, first run" from "the user's settings, unreadable."
  3. _ensure() skips the mirror write because _state is falsy, so the good mirror survives -- for now.
  4. The user changes one setting. set_* writes a one-key settings.json, then _mirror_settings() overwrites the good backup with that same one-key state.

Net: settings gone from both the file and its backup, with only a warning in the log.

A non-crash trigger reaches the same place: any transient read failure at step 2 (sharing violation, AV lock, permissions) also yields {}.

Why this is worse than it looks

jobs_dir loss is the severe case. config._stored_jobs_dir() falls back to the default and a relocated library looks empty -- precisely the failure _mirror_settings was written to prevent.

The Windows restore path cannot repair it either: migrate_persisted_files (desktop/src-tauri/src/main.rs:2771) copies from the mirror only if source.is_file() && !destination.exists(). A corrupt destination exists, so the safety net never fires. It covers "missing", not "corrupt".

Fix

Three separate changes, all in settings.py:

  1. _save(): same-directory temp + replace, matching _mirror_settings and registry.persist.
  2. _load(): distinguish a corrupt file from an absent one. Do not silently return {} for a file that exists but does not parse -- preserve it (rename aside) and prefer the mirror.
  3. Do not let a {} state overwrite a non-empty mirror.

Test

  • _save() leaves the previous file intact when interrupted mid-write.
  • _load() on a truncated file is distinguishable from _load() with no file.
  • A corrupt primary with a good mirror recovers the user's settings rather than resetting them.

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 workingreliabilityStability, resource management, race conditions

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions