You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The chapter download job and the "live read a not-yet-downloaded chapter" path both read/write the exact same per-chapter cache directory (getChapterCachePath), using the same file-naming convention, with no locking between them. This can result in either side observing/serving inconsistent, incomplete, or stale data when a chapter is downloaded and read at the same time.
Where this comes from
The download job (ChaptersFilesProvider.downloadImpl, called from Downloader.kt) writes each page into getChapterCachePath(mangaId, chapterId), then checks "does this page already exist" via ImageResponse.findFileNameStartingWith(cacheChapterDir, fileName) (a bare filename-prefix match) before deciding whether to (re)download/post-process a page.
The "live read" path for a chapter that isn't marked isDownloaded yet (Page.getPageImage()) fetches pages directly from the source and caches them into the same directory, using the same filename convention, via ImageResponse.getImageResponse() (which also does a prefix match to decide whether to serve a cached file or fetch fresh).
Neither path is aware of the other running concurrently for the same chapter. There is no mutex/lock coordinating them.
ChapterTable.pageCount / isDownloaded are also only reconciled with the real on-disk state lazily, the next time ChapterForDownload.getChapterDownloadReady() runs (e.g. the next fetchChapterPages call) - not live during an in-progress download.
How this gets triggered in practice (not just theoretical)
Confirmed while investigating this for #2288: the Suwayomi-Tsumiru client's "save chapter to device" flow (offline_download_providers.dart, saveChapterToDevice) deliberately fires both:
addChaptersBatchToDownloadQueue (enqueues the chapter on the server's own download queue), and
its own device-side page fetch (via the same fetchChapterPages mutation used for live reading)
... at the same time, explicitly not waiting on one before starting the other (their own code comment: "The device copy doesn't wait on it — the server streams pages from source meanwhile."). So this is the default behavior whenever a Tsumiru user downloads a chapter to their device that the server hasn't downloaded yet - not an edge case.
Concrete failure modes observed/reasoned through
The live-read cache lookup can match a file that's still mid-write (.tmp) or, in some post-processing scenarios, a file that doesn't represent a complete, standalone page - and serve it as if it were the final image.
The download job's own "page already downloaded" check can match a file written by a concurrent live-read (including an in-flight .tmp), causing it to silently skip that page - meaning any download-time post-processing for that page (e.g. format conversion via downloadConversions, or the tall-image splitting proposed in Add option to split tall images when downloading chapters #2288) never runs for it, with no error surfaced.
Clients that already had chapter metadata cached before a download finished may keep seeing a stale pageCount/isDownloaded state until they re-fetch chapter data.
Impact
No permanent data loss, but a live reader can receive wrong/incomplete image bytes, or a downloaded chapter can end up with some pages silently skipped by post-processing, whenever a chapter is read (WebUI, Tsumiru, or any other client) at the same time the server is downloading it.
Suggested direction
Some form of per-chapter mutual exclusion between the live single-page fetch path (Page.getPageImage) and the download job (ChaptersFilesProvider.downloadImpl), so they don't concurrently touch the same cache files for the same chapter. This is bigger in scope than a single feature PR, hence filing separately rather than folding a fix into #2288.
Found and analyzed while working on #2288 (split tall images on download); see that PR's discussion for more context.
Summary
The chapter download job and the "live read a not-yet-downloaded chapter" path both read/write the exact same per-chapter cache directory (
getChapterCachePath), using the same file-naming convention, with no locking between them. This can result in either side observing/serving inconsistent, incomplete, or stale data when a chapter is downloaded and read at the same time.Where this comes from
ChaptersFilesProvider.downloadImpl, called fromDownloader.kt) writes each page intogetChapterCachePath(mangaId, chapterId), then checks "does this page already exist" viaImageResponse.findFileNameStartingWith(cacheChapterDir, fileName)(a bare filename-prefix match) before deciding whether to (re)download/post-process a page.isDownloadedyet (Page.getPageImage()) fetches pages directly from the source and caches them into the same directory, using the same filename convention, viaImageResponse.getImageResponse()(which also does a prefix match to decide whether to serve a cached file or fetch fresh).ChapterTable.pageCount/isDownloadedare also only reconciled with the real on-disk state lazily, the next timeChapterForDownload.getChapterDownloadReady()runs (e.g. the nextfetchChapterPagescall) - not live during an in-progress download.How this gets triggered in practice (not just theoretical)
Confirmed while investigating this for #2288: the Suwayomi-Tsumiru client's "save chapter to device" flow (
offline_download_providers.dart,saveChapterToDevice) deliberately fires both:addChaptersBatchToDownloadQueue(enqueues the chapter on the server's own download queue), andfetchChapterPagesmutation used for live reading)... at the same time, explicitly not waiting on one before starting the other (their own code comment: "The device copy doesn't wait on it — the server streams pages from source meanwhile."). So this is the default behavior whenever a Tsumiru user downloads a chapter to their device that the server hasn't downloaded yet - not an edge case.
Concrete failure modes observed/reasoned through
.tmp) or, in some post-processing scenarios, a file that doesn't represent a complete, standalone page - and serve it as if it were the final image..tmp), causing it to silently skip that page - meaning any download-time post-processing for that page (e.g. format conversion viadownloadConversions, or the tall-image splitting proposed in Add option to split tall images when downloading chapters #2288) never runs for it, with no error surfaced.pageCount/isDownloadedstate until they re-fetch chapter data.Impact
No permanent data loss, but a live reader can receive wrong/incomplete image bytes, or a downloaded chapter can end up with some pages silently skipped by post-processing, whenever a chapter is read (WebUI, Tsumiru, or any other client) at the same time the server is downloading it.
Suggested direction
Some form of per-chapter mutual exclusion between the live single-page fetch path (
Page.getPageImage) and the download job (ChaptersFilesProvider.downloadImpl), so they don't concurrently touch the same cache files for the same chapter. This is bigger in scope than a single feature PR, hence filing separately rather than folding a fix into #2288.Found and analyzed while working on #2288 (split tall images on download); see that PR's discussion for more context.