Fix infinite preload setting and race condition in imageloader

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()
         }
     }
 

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")
         }

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
         }

ultrasonic/src/main/res/xml/settings.xml

@@ -222,7 +222,7 @@
             a:title="@string/settings.cache_location"
             app:iconSpaceReserved="false"/>
         <ListPreference
-            a:defaultValue="3"
+            a:defaultValue="5"
             a:entries="@array/preloadCountNames"
             a:entryValues="@array/preloadCountValues"
             a:key="@string/setting_key.preload_count"