From 0420a2bf7bd2f10cefef3f8fbbf4fee31a603d9f Mon Sep 17 00:00:00 2001 From: birdbird <6892457-bird.bird@users.noreply.gitlab.com> Date: Thu, 1 Oct 2026 21:48:46 +0000 Subject: [PATCH] Fix infinite preload setting and race condition in imageloader --- .../ultrasonic/imageloader/ImageLoader.kt | 81 ++++++++++++------- .../provider/AlbumArtContentProvider.kt | 9 ++- .../org/moire/ultrasonic/util/Settings.kt | 2 +- ultrasonic/src/main/res/xml/settings.xml | 2 +- 4 files changed, 59 insertions(+), 35 deletions(-) diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/imageloader/ImageLoader.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/imageloader/ImageLoader.kt index 6e2f1602..da52e610 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/imageloader/ImageLoader.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/imageloader/ImageLoader.kt @@ -28,6 +28,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.guava.future import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext import org.moire.ultrasonic.R import org.moire.ultrasonic.api.subsonic.SubsonicAPIClient @@ -192,50 +193,70 @@ class ImageLoader( fun downloadCoverArt(id: String, file: String) = launch { if (id.isBlank()) return@launch - withContext(Dispatchers.IO) { - // Return if have a cache hit - if (File(file).exists()) return@withContext + downloadCoverArtInternal(id, file) + } + } - // If another coroutine has started caching, abort - if (cacheInProgress[file] != null) return@withContext + /** + * Blocking variant of [downloadCoverArt] for callers that need the file to exist (or + * the download to have failed) when the call returns, e.g. + * AlbumArtContentProvider.openFile on a Binder thread. Waits for an already running + * download of the same file instead of starting a duplicate request. + */ + fun downloadCoverArtBlocking(id: String, file: String) { + if (id.isBlank()) return + runBlocking { + withContext(Dispatchers.IO) { + downloadCoverArtInternal(id, file) + } + } + } + private suspend fun downloadCoverArtInternal(id: String, file: String) { + // Return if have a cache hit + if (File(file).exists()) return + + // If another coroutine has started caching this file, wait for it to finish + val alreadyInProgress = cacheInProgress.putIfAbsent(file, CountDownLatch(1)) + if (alreadyInProgress != null) { + alreadyInProgress.await() + return + } + + try { // Always download the large size.. val size = config.largeSize - File(file).createNewFile() - // Query the API Timber.d("Loading cover art for: %s", id) + val response = API.getCoverArt(id, size.toLong()).execute().toStreamResponse() + + response.throwOnFailure() + + // Check for failure + if (response.stream == null) return + + // Write Response stream to file + var inputStream: InputStream? = null try { - val response = API.getCoverArt(id, size.toLong()).execute().toStreamResponse() - - response.throwOnFailure() - - // Check for failure - if (response.stream == null) return@withContext - - // Write Response stream to file - var inputStream: InputStream? = null + inputStream = response.stream + val bytes = inputStream!!.readBytes() + var outputStream: OutputStream? = null try { - inputStream = response.stream - val bytes = inputStream!!.readBytes() - var outputStream: OutputStream? = null - try { - outputStream = FileOutputStream(file) - outputStream.write(bytes) - } finally { - outputStream.safeClose() - } + outputStream = FileOutputStream(file) + outputStream.write(bytes) } finally { - inputStream.safeClose() + outputStream.safeClose() } - } catch (all: Exception) { - Timber.w(all) - runCatching { File(file).delete() } } finally { - cacheInProgress.remove(file)?.countDown() + inputStream.safeClose() } + } catch (all: Exception) { + Timber.w(all) + runCatching { File(file).delete() } + } finally { + cacheInProgress.remove(file)?.countDown() } } diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/provider/AlbumArtContentProvider.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/provider/AlbumArtContentProvider.kt index 424e00d2..58fb7151 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/provider/AlbumArtContentProvider.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/provider/AlbumArtContentProvider.kt @@ -67,9 +67,12 @@ class AlbumArtContentProvider : // called from other processes before Koin was started. In that case serve the file // from the cache, but don't trigger a download. if (GlobalContext.getOrNull() != null) { - imageLoaderProvider.executeOn { - it.downloadCoverArt(parts[0], albumArtFile) - } + // Download synchronously: openFile runs on a Binder thread and callers expect + // the file to exist (or the request to have failed) when this returns. A + // fire-and-forget download made callers re-query and re-trigger the download, + // spamming the server with getCoverArt requests. + imageLoaderProvider.getImageLoader() + .downloadCoverArtBlocking(parts[0], albumArtFile) } else { Timber.w("AlbumArtContentProvider queried before Koin started, serving cache only") } diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/Settings.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/Settings.kt index c7debffd..f8c5d130 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/Settings.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/Settings.kt @@ -56,7 +56,7 @@ object Settings { val preloadCount: Int get() { val preloadCount = - preferences.getString(getKey(R.string.setting_key_preload_count), "-1")!! + preferences.getString(getKey(R.string.setting_key_preload_count), "5")!! .toInt() return if (preloadCount == -1) Int.MAX_VALUE else preloadCount } diff --git a/ultrasonic/src/main/res/xml/settings.xml b/ultrasonic/src/main/res/xml/settings.xml index 9883c560..b39c6b17 100644 --- a/ultrasonic/src/main/res/xml/settings.xml +++ b/ultrasonic/src/main/res/xml/settings.xml @@ -222,7 +222,7 @@ a:title="@string/settings.cache_location" app:iconSpaceReserved="false"/>