Skip to content

Episode reload can overwrite in-flight download state (refresh and initial load) #19

Open

Description

@AlexAxthelm

Summary

Reloading a feed's episodes replaces model.episodes wholesale with the database rows. Download-state changes are applied to model.episodes immediately but written to the database through a queue, so a reload whose read was queued before a state change can arrive after it and revert the row. The window is one storage read (milliseconds), so this is rare, but the visible effects are wrong and don't self-correct.

Found while reviewing the feed refresh work (PR #18). The reload after a refresh made it easier to hit, but the same hole exists in the initial load in SelectSubscription. It was not reproduced; this comes from reading the code.

How it happens

  1. A refresh finishes for the feed on screen. RefreshSaved issues ListEpisodesBySubscription; call this read R.
  2. Before R's result is delivered, a download event changes an episode: DownloadFinished -> set_download_state(Downloaded), or the user taps Download -> set_download_state(Queued). The in-memory row changes at once; the UpdateDownloadState write is queued behind R on the serial storage stream.
  3. R executes against the database before that write, so it returns the old status.
  4. EpisodesLoaded replaces model.episodes with R's rows, reverting the row.
  5. The write lands afterwards, so the database is right and the UI is wrong.

Impact

  • A finished download shows as "Downloading" with no progress, or a just-queued one shows as "Not downloaded".
  • SelectSubscription is idempotent, so nothing reloads the screen until the user leaves the feed and returns, or restarts the app.
  • DownloadEpisode accepts any NotDownloaded/Failed row and pushes onto download_queue without checking whether the episode is already queued or downloading. A second tap on a reverted row can enqueue a duplicate.

Options

  1. Do nothing and document it. No risk; the rare glitch remains.
  2. Epoch guard with a re-query (recommended). Keep a counter in the model that set_download_state bumps, and record its value when a load is requested. If it changed when the result arrives, drop the result and request the load again; the storage stream is serial, so the second read sees the pending write. Covers the refresh reload and the initial SelectSubscription load, needs no FFI change, and keeps the database as the source of truth so RemovedFromFeed flags from a refresh still show. Costs an extra query in the rare case and needs a retry cap.
  3. Replay a log of transitions over the loaded rows. Record download-state changes while a load is in flight and re-apply them to the result. No re-query, but the most new state to manage. The only option that avoids a re-query for the initial load, where model.episodes is empty and there is nothing to overlay from.
  4. Prefer in-memory download fields when merging. Rejected as stated: a refresh changes download_status in the database itself (RemovedFromFeed, revived episodes) and the core can't know, so this would hide what the reload is for. It only works if limited to episodes the core has touched, which is option 3.

Independent of the choice, DownloadEpisode should ignore an episode that is already in download_queue or is downloading, whatever its displayed status. About three lines, and it removes the duplicate-enqueue consequence even if a stale row slips through.

Suggested plan

Option 2 plus the DownloadEpisode guard, as separate commits.

Tests can be deterministic by sequencing core events, with no timing:

  • RefreshSaved for the open feed, then DownloadFinished for one of its episodes, then a stale EpisodesLoaded. Expect the stale result to be dropped, a new ListEpisodesBySubscription to be requested, and the row to stay Downloaded throughout.
  • The same sequence around SelectSubscription's initial load.
  • A reload with no intervening transition is applied normally, with no extra request.
  • Repeated transitions stop at the retry cap rather than looping.

Where

  • shared/src/app.rs: the RefreshSaved and SelectSubscription handlers (both issue ListEpisodesBySubscription), EpisodesLoaded, set_download_state (the single place in-memory download state changes), and DownloadEpisode.
  • docs/features/subscription.md: the note about SelectSubscription being idempotent.

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

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions