[PR #1833] [CLOSED] Android: improve download throughput, safety, and concurrency #1715

New Issue

2026-04-25T00:00:48+02:00

adam commented

2026-04-25 00:00:48 +02:00

📋 Pull Request Information

Original PR: https://github.com/advplyr/audiobookshelf-app/pull/1833
Author: @ricetim
Created: 3/28/2026
Status: ❌ Closed

Base: master ← Head: improve/download-perf-on-1587

📝 Commits (10+)

feef527 Delet unused file
ee0ab43 Auto formatting AbsDownloader.kt
4a09238 Refactor AbsDownloader
19b1f3b Remove: unused json file from FolderScanner
5d8574e Simplify local library item creation in folder scanner
f31a8f0 Revert "Refactor AbsDownloader"
841527e Simplify AbsDownloader download item creation
640d8cd Combine cover download logic
7e2ac27 Fix: cover printout for book
e11b8cb Initial internal download continue incomplete download

📊 Changes

8 files changed (+811 additions, -868 deletions)

View changed files

📝 android/app/src/main/java/com/audiobookshelf/app/data/DeviceClasses.kt (+133 -113)
➖ android/app/src/main/java/com/audiobookshelf/app/data/FolderScanResult.kt (+0 -15)
📝 android/app/src/main/java/com/audiobookshelf/app/device/FolderScanner.kt (+189 -415)
📝 android/app/src/main/java/com/audiobookshelf/app/managers/DownloadItemManager.kt (+103 -135)
📝 android/app/src/main/java/com/audiobookshelf/app/managers/InternalDownloadManager.kt (+104 -47)
📝 android/app/src/main/java/com/audiobookshelf/app/models/DownloadItemPart.kt (+69 -50)
📝 android/app/src/main/java/com/audiobookshelf/app/plugins/AbsDownloader.kt (+212 -92)
📝 strings/en-us.json (+1 -1)

📄 Description

Brief summary

Improves Android download speed, correctness, and concurrent download behaviour. This PR is based on top of #1587 and is intended to complement that refactor — if #1587 merges first, the diff here collapses to just the two files changed in the final commit.

Which issue is fixed?

Related to #1167, #1428, #1479 (same issues referenced in #1587).

Pull Request Type

Android backend only. No frontend or iOS changes.

In-depth Description

All changes are in InternalDownloadManager.kt and DownloadItemManager.kt.

`InternalDownloadManager`

Shared OkHttpClient singleton
Previously a new OkHttpClient was constructed per InternalDownloadManager instance, so each file download paid a fresh TLS handshake cost even when downloading multiple files from the same server. Moving it to a companion object with a ConnectionPool(5) reuses TCP connections across files.

readTimeout(30s)
Without a read timeout, if a server stops sending data while keeping the TCP connection open (e.g. a hung reverse proxy), the OkHttp dispatcher thread blocks on inputStream.read() indefinitely — even after the stall detector in DownloadItemManager has already removed the part from the active list. 30 seconds is generous enough that even very slow connections never false-fire (since read() returns as soon as any bytes arrive, not when the full buffer is filled), while ensuring the thread is eventually released via SocketTimeoutException.

Buffer 8 KiB → 256 KiB, remove BufferedInputStream
OkHttp/Okio already provides internal buffering on the response body stream; wrapping it in a BufferedInputStream with an 8 KiB window adds a redundant copy. A 256 KiB read buffer reduces the number of read()/write() syscall pairs by 32× for large audio files, which is the dominant overhead on a fast LAN connection.

Catch IOException inside write()
The outer try/catch(Exception) in download() wraps the synchronous setup before enqueue(), not the async callback. If outputStream.write() throws (e.g. disk full mid-transfer), the exception propagates through the OkHttp dispatcher which catches and logs it silently — progressCallback.onComplete() is never called and the download slot is permanently leaked, preventing any further downloads from starting. Catching IOException inside write() ensures onComplete(true) always fires.

Call onProgress every chunk — no throttling
The Capacitor bridge is crossed in the watcher loop on its own 500ms schedule, not in onProgress. Throttling onProgress would starve lastUpdateTime updates and cause the stall detector (introduced in #1587) to false-fire mid-transfer at moderate download speeds.

`DownloadItemManager`

CopyOnWriteArrayList for both lists
mutableListOf() is not thread-safe. The watcher coroutine iterates currentDownloadItemParts while OkHttp callback threads and Dispatchers.Main coroutines call add()/remove(), which can produce ConcurrentModificationException in practice when downloading many files simultaneously.

@Volatile isDownloading
Without @Volatile, the JIT can cache the value in a register. A coroutine that sets isDownloading = false at the end of the watcher loop may never have that write seen by the thread checking it in startWatchingDownloads, causing a second watcher to silently never launch after the first one exits.

@Synchronized on addDownloadItem and checkUpdateDownloadQueue
When a large series (e.g. 90 files) is queued, metadata callbacks for all 90 items can complete near-simultaneously, causing multiple threads to call checkUpdateDownloadQueue concurrently. Each thread sees downloadId == null for the same DownloadItemPart and starts a duplicate InternalDownloadManager for it — two OkHttp calls writing interleaved bytes to the same file, corrupting it, while progress oscillates as two independent byte counters advance.

Round-robin slot distribution
With sequential slot filling, all available download slots are assigned to the first queued book before any slot is given to the second. Round-robin distributes one slot per book per pass, so all queued books make progress simultaneously.

How have you tested this?

Built the APK from this branch using GitHub Actions and sideloaded it onto an Android device. Downloaded a large multi-file audiobook series over LAN from a local Audiobookshelf instance and verified that:

Multiple files download concurrently
Downloads complete correctly without corruption
Progress reporting is stable (no backwards jumps)
The full series completes without hanging at the "Processing" step

Screenshots

N/A — backend only changes.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/advplyr/audiobookshelf-app/pull/1833 **Author:** [@ricetim](https://github.com/ricetim) **Created:** 3/28/2026 **Status:** ❌ Closed **Base:** `master` ← **Head:** `improve/download-perf-on-1587` --- ### 📝 Commits (10+) - [`feef527`](https://github.com/advplyr/audiobookshelf-app/commit/feef527bede069b06ecb0b39001b3ec1e3d5fe47) Delet unused file - [`ee0ab43`](https://github.com/advplyr/audiobookshelf-app/commit/ee0ab43fc57d5d23bed80f6be1f353402df896ab) Auto formatting `AbsDownloader.kt` - [`4a09238`](https://github.com/advplyr/audiobookshelf-app/commit/4a09238a8e13409e4052b4bfb0c42843b46d333d) Refactor `AbsDownloader` - [`19b1f3b`](https://github.com/advplyr/audiobookshelf-app/commit/19b1f3b6317b80cf70c56f77f76cbc5fe59c29ba) Remove: unused json file from FolderScanner - [`5d8574e`](https://github.com/advplyr/audiobookshelf-app/commit/5d8574eb8712fc73bf292fe426aa2502f1395d5b) Simplify local library item creation in folder scanner - [`f31a8f0`](https://github.com/advplyr/audiobookshelf-app/commit/f31a8f04825a7bc39854e4f1355f63c8411efa85) Revert "Refactor `AbsDownloader`" - [`841527e`](https://github.com/advplyr/audiobookshelf-app/commit/841527eb74ef28d88fe4873b36bbc5b4c40867c7) Simplify `AbsDownloader` download item creation - [`640d8cd`](https://github.com/advplyr/audiobookshelf-app/commit/640d8cdd4103942c2a52cbb6622d0b3537d9f7c5) Combine cover download logic - [`7e2ac27`](https://github.com/advplyr/audiobookshelf-app/commit/7e2ac27eba9f717249318bf88f0690b183c01bad) Fix: cover printout for book - [`e11b8cb`](https://github.com/advplyr/audiobookshelf-app/commit/e11b8cb66ce97c4b047104e7849d864035c81a4e) Initial internal download continue incomplete download ### 📊 Changes **8 files changed** (+811 additions, -868 deletions) <details> <summary>View changed files</summary> 📝 `android/app/src/main/java/com/audiobookshelf/app/data/DeviceClasses.kt` (+133 -113) ➖ `android/app/src/main/java/com/audiobookshelf/app/data/FolderScanResult.kt` (+0 -15) 📝 `android/app/src/main/java/com/audiobookshelf/app/device/FolderScanner.kt` (+189 -415) 📝 `android/app/src/main/java/com/audiobookshelf/app/managers/DownloadItemManager.kt` (+103 -135) 📝 `android/app/src/main/java/com/audiobookshelf/app/managers/InternalDownloadManager.kt` (+104 -47) 📝 `android/app/src/main/java/com/audiobookshelf/app/models/DownloadItemPart.kt` (+69 -50) 📝 `android/app/src/main/java/com/audiobookshelf/app/plugins/AbsDownloader.kt` (+212 -92) 📝 `strings/en-us.json` (+1 -1) </details> ### 📄 Description ## Brief summary Improves Android download speed, correctness, and concurrent download behaviour. This PR is based on top of #1587 and is intended to complement that refactor — if #1587 merges first, the diff here collapses to just the two files changed in the final commit. ## Which issue is fixed? Related to #1167, #1428, #1479 (same issues referenced in #1587). ## Pull Request Type Android backend only. No frontend or iOS changes. ## In-depth Description All changes are in `InternalDownloadManager.kt` and `DownloadItemManager.kt`. ### `InternalDownloadManager` **Shared `OkHttpClient` singleton** Previously a new `OkHttpClient` was constructed per `InternalDownloadManager` instance, so each file download paid a fresh TLS handshake cost even when downloading multiple files from the same server. Moving it to a `companion object` with a `ConnectionPool(5)` reuses TCP connections across files. **`readTimeout(30s)`** Without a read timeout, if a server stops sending data while keeping the TCP connection open (e.g. a hung reverse proxy), the OkHttp dispatcher thread blocks on `inputStream.read()` indefinitely — even after the stall detector in `DownloadItemManager` has already removed the part from the active list. 30 seconds is generous enough that even very slow connections never false-fire (since `read()` returns as soon as any bytes arrive, not when the full buffer is filled), while ensuring the thread is eventually released via `SocketTimeoutException`. **Buffer 8 KiB → 256 KiB, remove `BufferedInputStream`** OkHttp/Okio already provides internal buffering on the response body stream; wrapping it in a `BufferedInputStream` with an 8 KiB window adds a redundant copy. A 256 KiB read buffer reduces the number of `read()`/`write()` syscall pairs by 32× for large audio files, which is the dominant overhead on a fast LAN connection. **Catch `IOException` inside `write()`** The outer `try/catch(Exception)` in `download()` wraps the synchronous setup before `enqueue()`, not the async callback. If `outputStream.write()` throws (e.g. disk full mid-transfer), the exception propagates through the OkHttp dispatcher which catches and logs it silently — `progressCallback.onComplete()` is never called and the download slot is permanently leaked, preventing any further downloads from starting. Catching `IOException` inside `write()` ensures `onComplete(true)` always fires. **Call `onProgress` every chunk — no throttling** The Capacitor bridge is crossed in the watcher loop on its own 500ms schedule, not in `onProgress`. Throttling `onProgress` would starve `lastUpdateTime` updates and cause the stall detector (introduced in #1587) to false-fire mid-transfer at moderate download speeds. ### `DownloadItemManager` **`CopyOnWriteArrayList` for both lists** `mutableListOf()` is not thread-safe. The watcher coroutine iterates `currentDownloadItemParts` while OkHttp callback threads and `Dispatchers.Main` coroutines call `add()`/`remove()`, which can produce `ConcurrentModificationException` in practice when downloading many files simultaneously. **`@Volatile isDownloading`** Without `@Volatile`, the JIT can cache the value in a register. A coroutine that sets `isDownloading = false` at the end of the watcher loop may never have that write seen by the thread checking it in `startWatchingDownloads`, causing a second watcher to silently never launch after the first one exits. **`@Synchronized` on `addDownloadItem` and `checkUpdateDownloadQueue`** When a large series (e.g. 90 files) is queued, metadata callbacks for all 90 items can complete near-simultaneously, causing multiple threads to call `checkUpdateDownloadQueue` concurrently. Each thread sees `downloadId == null` for the same `DownloadItemPart` and starts a duplicate `InternalDownloadManager` for it — two OkHttp calls writing interleaved bytes to the same file, corrupting it, while progress oscillates as two independent byte counters advance. **Round-robin slot distribution** With sequential slot filling, all available download slots are assigned to the first queued book before any slot is given to the second. Round-robin distributes one slot per book per pass, so all queued books make progress simultaneously. ## How have you tested this? Built the APK from this branch using GitHub Actions and sideloaded it onto an Android device. Downloaded a large multi-file audiobook series over LAN from a local Audiobookshelf instance and verified that: - Multiple files download concurrently - Downloads complete correctly without corruption - Progress reporting is stable (no backwards jumps) - The full series completes without hanging at the "Processing" step ## Screenshots N/A — backend only changes. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

adam added the pull-request label 2026-04-25 00:00:48 +02:00

adam closed this issue

2026-04-25 00:00:48 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/audiobookshelf-app#1715