Repository navigation
Episode reload can overwrite in-flight download state (refresh and initial load) #19
Description
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
- A refresh finishes for the feed on screen.
RefreshSavedissuesListEpisodesBySubscription; call this read R. - 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; theUpdateDownloadStatewrite is queued behind R on the serial storage stream. - R executes against the database before that write, so it returns the old status.
EpisodesLoadedreplacesmodel.episodeswith R's rows, reverting the row.- 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".
SelectSubscriptionis idempotent, so nothing reloads the screen until the user leaves the feed and returns, or restarts the app.DownloadEpisodeaccepts anyNotDownloaded/Failedrow and pushes ontodownload_queuewithout checking whether the episode is already queued or downloading. A second tap on a reverted row can enqueue a duplicate.
Options
- Do nothing and document it. No risk; the rare glitch remains.
- Epoch guard with a re-query (recommended). Keep a counter in the model that
set_download_statebumps, 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 initialSelectSubscriptionload, needs no FFI change, and keeps the database as the source of truth soRemovedFromFeedflags from a refresh still show. Costs an extra query in the rare case and needs a retry cap. - 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.episodesis empty and there is nothing to overlay from. - Prefer in-memory download fields when merging. Rejected as stated: a refresh changes
download_statusin 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:
RefreshSavedfor the open feed, thenDownloadFinishedfor one of its episodes, then a staleEpisodesLoaded. Expect the stale result to be dropped, a newListEpisodesBySubscriptionto be requested, and the row to stayDownloadedthroughout.- 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: theRefreshSavedandSelectSubscriptionhandlers (both issueListEpisodesBySubscription),EpisodesLoaded,set_download_state(the single place in-memory download state changes), andDownloadEpisode.docs/features/subscription.md: the note aboutSelectSubscriptionbeing idempotent.
Activity