⚡ Bolt: [replace blocking future.get() inside async future listener with Guava's Futures.addCallback]

.jules/bolt.md

@@ -4,3 +4,6 @@
 ## 2024-05-19 - Replacing pre-sized ArrayList + for loop with List(size) constructor
 **Learning:** In Kotlin, creating a `List` using the functional constructor `List(size) { index -> ... }` can be slightly faster and is definitely cleaner than manually sizing an `ArrayList` and using a `for` loop to `.add()` items, even when the `ArrayList` is pre-sized.
 **Action:** Default to the `List(size) { ... }` constructor when mapping indexed access (like from an Android framework class or external API that doesn't provide an Iterator) into a Kotlin List.
+## 2025-07-31 - Replacing blocking future.get() inside async future listener with Guava's Futures.addCallback
+**Learning:** Using `future.get()` inside a `future.addListener` (which is standard practice to wait for `ListenableFuture` resolution if it's guaranteed to be done by the time the listener is invoked) is technically safe from deadlock. However, because it still involves a `.get()` call, it can cause the execution thread to block synchronously. In components where the callback executor is the main UI thread (e.g. `ContextCompat.getMainExecutor`), this can cause subtle UI janks or StrictMode violations.
+**Action:** Always prefer non-blocking async idioms for future completion on UI threads. Use Guava's `Futures.addCallback` to register success and failure callbacks rather than `addListener` combined with a blocking `get()`.

app/src/main/java/com/rockmusic/app/player/PlayerConnection.kt

@@ -12,6 +12,8 @@ import androidx.media3.common.PlaybackException
 import androidx.media3.common.Player
 import androidx.media3.session.MediaController
 import androidx.media3.session.SessionToken
+import com.google.common.util.concurrent.FutureCallback
+import com.google.common.util.concurrent.Futures
 import com.rockmusic.app.domain.model.LocalTrack
 import dagger.hilt.android.qualifiers.ApplicationContext
 import javax.inject.Inject
@@ -66,31 +68,35 @@ class PlayerConnection @Inject constructor(
     init {
         val token = SessionToken(context, ComponentName(context, MusicService::class.java))
         val future = MediaController.Builder(context, token).buildAsync()
-        future.addListener(
-            {
-                runCatching { future.get() }
-                    .onSuccess { mediaController ->
-                        controller = mediaController
-                        mediaController.addListener(listener)
-                        pendingVolume?.let { requestedVolume ->
-                            mediaController.volume = requestedVolume
-                            pendingVolume = null
-                        }
-                        val queue = pendingQueue
-                        if (queue == null) {
-                            publish(mediaController, includeQueue = true)
-                        } else {
-                            pendingQueue = null
-                            startPlayback(mediaController, queue.tracks, queue.startIndex)
-                        }
-                        startPositionUpdates()
+        // ⚡ Bolt: Replaced blocking future.get() inside listener with Guava's Futures.addCallback
+        // to avoid blocking the main UI thread executor when waiting for the MediaController setup.
+        Futures.addCallback(
+            future,
+            object : FutureCallback<MediaController> {
+                override fun onSuccess(mediaController: MediaController?) {
+                    if (mediaController == null) return
+                    controller = mediaController
+                    mediaController.addListener(listener)
+                    pendingVolume?.let { requestedVolume ->
+                        mediaController.volume = requestedVolume
+                        pendingVolume = null
                     }
-                    .onFailure { error ->
-                        _state.value = _state.value.copy(
-                            isPreparing = false,
-                            errorMessage = error.message ?: "Unable to start the audio player.",
-                        )
+                    val queue = pendingQueue
+                    if (queue == null) {
+                        publish(mediaController, includeQueue = true)
+                    } else {
+                        pendingQueue = null
+                        startPlayback(mediaController, queue.tracks, queue.startIndex)
                     }
+                    startPositionUpdates()
+                }
+
+                override fun onFailure(error: Throwable) {
+                    _state.value = _state.value.copy(
+                        isPreparing = false,
+                        errorMessage = error.message ?: "Unable to start the audio player.",
+                    )
+                }
             },
             ContextCompat.getMainExecutor(context),
         )

app/src/test/java/com/rockmusic/app/player/PlayerConnectionInitBenchmark.kt

@@ -0,0 +1,43 @@
+package com.rockmusic.app.player
+
+import android.content.Context
+import androidx.test.core.app.ApplicationProvider
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.StandardTestDispatcher
+import kotlinx.coroutines.test.resetMain
+import kotlinx.coroutines.test.setMain
+import org.junit.After
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+import kotlin.system.measureTimeMillis
+
+@OptIn(ExperimentalCoroutinesApi::class)
+@RunWith(RobolectricTestRunner::class)
+@Config(sdk = [34])
+class PlayerConnectionInitBenchmark {
+    private val testDispatcher = StandardTestDispatcher()
+
+    @Before
+    fun setup() {
+        Dispatchers.setMain(testDispatcher)
+    }
+
+    @After
+    fun tearDown() {
+        Dispatchers.resetMain()
+    }
+
+    @Test
+    fun benchmarkInit() {
+        val context = ApplicationProvider.getApplicationContext<Context>()
+        val time = measureTimeMillis {
+            // Note: In Robolectric, MediaController might not actually bind, but we measure the init path
+            PlayerConnection(context)
+        }
+        println("PlayerConnection init took $time ms")
+    }
+}