From 736f3ec6b7266c615251eac5debdfa478414d065 Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Sat, 3 Jun 2023 09:04:44 -0600 Subject: [PATCH] playback: fix crash on state restore Fix a crash stemming from applying the playback state on the main thread instead of the background thread. all: add misc todos --- app/src/main/java/org/oxycblt/auxio/MainActivity.kt | 2 +- .../oxycblt/auxio/list/adapter/PlayingIndicatorAdapter.kt | 7 ++++--- .../main/java/org/oxycblt/auxio/music/user/UserLibrary.kt | 1 + .../org/oxycblt/auxio/playback/PlaybackPanelFragment.kt | 2 -- .../org/oxycblt/auxio/playback/system/PlaybackService.kt | 5 ++++- 5 files changed, 10 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/oxycblt/auxio/MainActivity.kt b/app/src/main/java/org/oxycblt/auxio/MainActivity.kt index a5df4f5b4..725f60444 100644 --- a/app/src/main/java/org/oxycblt/auxio/MainActivity.kt +++ b/app/src/main/java/org/oxycblt/auxio/MainActivity.kt @@ -51,7 +51,7 @@ import org.oxycblt.auxio.util.systemBarInsetsCompat * TODO: Unit testing * TODO: Fix UID naming * TODO: Leverage FlexibleListAdapter more in dialogs (Disable item anims) - * TODO: Try to move on from synchronized and volatile in shared objs + * TODO: Improve multi-threading support in shared objects */ @AndroidEntryPoint class MainActivity : AppCompatActivity() { diff --git a/app/src/main/java/org/oxycblt/auxio/list/adapter/PlayingIndicatorAdapter.kt b/app/src/main/java/org/oxycblt/auxio/list/adapter/PlayingIndicatorAdapter.kt index ab0db5fe3..1beddc8ef 100644 --- a/app/src/main/java/org/oxycblt/auxio/list/adapter/PlayingIndicatorAdapter.kt +++ b/app/src/main/java/org/oxycblt/auxio/list/adapter/PlayingIndicatorAdapter.kt @@ -22,6 +22,7 @@ import android.view.View import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.RecyclerView import org.oxycblt.auxio.util.logD +import org.oxycblt.auxio.util.logW /** * A [RecyclerView.Adapter] that supports indicating the playback status of a particular item. @@ -71,7 +72,7 @@ abstract class PlayingIndicatorAdapter( if (pos > -1) { notifyItemChanged(pos, PAYLOAD_PLAYING_INDICATOR_CHANGED) } else { - logD("oldItem was not in adapter data") + logW("oldItem was not in adapter data") } } @@ -81,7 +82,7 @@ abstract class PlayingIndicatorAdapter( if (pos > -1) { notifyItemChanged(pos, PAYLOAD_PLAYING_INDICATOR_CHANGED) } else { - logD("newItem was not in adapter data") + logW("newItem was not in adapter data") } } @@ -99,7 +100,7 @@ abstract class PlayingIndicatorAdapter( if (pos > -1) { notifyItemChanged(pos, PAYLOAD_PLAYING_INDICATOR_CHANGED) } else { - logD("newItem was not in adapter data") + logW("newItem was not in adapter data") } } } diff --git a/app/src/main/java/org/oxycblt/auxio/music/user/UserLibrary.kt b/app/src/main/java/org/oxycblt/auxio/music/user/UserLibrary.kt index f6a888eb7..412b14fa4 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/user/UserLibrary.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/user/UserLibrary.kt @@ -161,6 +161,7 @@ private class UserLibraryImpl( override fun findPlaylist(name: String) = playlistMap.values.find { it.name.raw == name } override suspend fun createPlaylist(name: String, songs: List) { + // TODO: Use synchronized with value access too val playlistImpl = PlaylistImpl.from(name, songs, musicSettings) synchronized(this) { playlistMap[playlistImpl.uid] = playlistImpl } val rawPlaylist = diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt index abd7aafbc..6bcc4114e 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt @@ -236,13 +236,11 @@ class PlaybackPanelFragment : requireBinding().playbackShuffle.isActivated = isShuffled } - /** Navigate to one of the currently playing [Song]'s Artists. */ private fun navigateToCurrentArtist() { val song = playbackModel.song.value ?: return navModel.exploreNavigateToParentArtist(song) } - /** Navigate to the currently playing [Song]'s albums. */ private fun navigateToCurrentAlbum() { val song = playbackModel.song.value ?: return navModel.exploreNavigateTo(song.album) diff --git a/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt b/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt index ab95fba78..848d47b4d 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt @@ -44,6 +44,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import org.oxycblt.auxio.BuildConfig import org.oxycblt.auxio.music.MusicRepository import org.oxycblt.auxio.music.MusicSettings @@ -355,7 +356,9 @@ class PlaybackService : logD("Restoring playback state") restoreScope.launch { persistenceRepository.readState()?.let { - playbackManager.applySavedState(it, false) + // Apply the saved state on the main thread to prevent code expecting + // state updates on the main thread from crashing. + withContext(Dispatchers.Main) { playbackManager.applySavedState(it, false) } } } }