From 8953f12a1e4912ef2bbd68067c2684e97b97e636 Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Sun, 21 May 2023 09:53:07 -0600 Subject: [PATCH] music: try to fix repo race conditions Forgot to slather the entire class in Synchronized and Volatile. Should make crashes less likely, I hope. --- .../java/org/oxycblt/auxio/MainActivity.kt | 1 + .../java/org/oxycblt/auxio/home/tabs/Tab.kt | 2 + .../oxycblt/auxio/music/MusicRepository.kt | 52 +++++++++---------- 3 files changed, 27 insertions(+), 28 deletions(-) diff --git a/app/src/main/java/org/oxycblt/auxio/MainActivity.kt b/app/src/main/java/org/oxycblt/auxio/MainActivity.kt index 6d9bc9e9b..9d87a0c68 100644 --- a/app/src/main/java/org/oxycblt/auxio/MainActivity.kt +++ b/app/src/main/java/org/oxycblt/auxio/MainActivity.kt @@ -51,6 +51,7 @@ import org.oxycblt.auxio.util.systemBarInsetsCompat * TODO: Fix UID naming * TODO: Leverage FlexibleListAdapter more in dialogs (Disable item anims) * TODO: Add more logging + * TODO: Try to move on from shared objs in synchronized and volatile */ @AndroidEntryPoint class MainActivity : AppCompatActivity() { diff --git a/app/src/main/java/org/oxycblt/auxio/home/tabs/Tab.kt b/app/src/main/java/org/oxycblt/auxio/home/tabs/Tab.kt index 30425e6d8..cd7bf8f06 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/tabs/Tab.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/tabs/Tab.kt @@ -26,6 +26,8 @@ import org.oxycblt.auxio.util.logE * * @param mode The type of list in the home view this instance corresponds to. * @author Alexander Capehart (OxygenCobalt) + * + * TODO: Tab migration to playlists is busted and resets the config entirely. Need to fix. */ sealed class Tab(open val mode: MusicMode) { /** diff --git a/app/src/main/java/org/oxycblt/auxio/music/MusicRepository.kt b/app/src/main/java/org/oxycblt/auxio/music/MusicRepository.kt index df5e011a3..6fa0b4f79 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicRepository.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicRepository.kt @@ -219,12 +219,12 @@ constructor( ) : MusicRepository { private val updateListeners = mutableListOf() private val indexingListeners = mutableListOf() - private var indexingWorker: MusicRepository.IndexingWorker? = null + @Volatile private var indexingWorker: MusicRepository.IndexingWorker? = null - override var deviceLibrary: DeviceLibrary? = null - override var userLibrary: MutableUserLibrary? = null - private var previousCompletedState: IndexingState.Completed? = null - private var currentIndexingState: IndexingState? = null + @Volatile override var deviceLibrary: DeviceLibrary? = null + @Volatile override var userLibrary: MutableUserLibrary? = null + @Volatile private var previousCompletedState: IndexingState.Completed? = null + @Volatile private var currentIndexingState: IndexingState? = null override val indexingState: IndexingState? get() = currentIndexingState ?: previousCompletedState @@ -272,55 +272,50 @@ constructor( currentIndexingState = null } + @Synchronized override fun find(uid: Music.UID) = (deviceLibrary?.run { findSong(uid) ?: findAlbum(uid) ?: findArtist(uid) ?: findGenre(uid) } ?: userLibrary?.findPlaylist(uid)) override suspend fun createPlaylist(name: String, songs: List) { - val userLibrary = userLibrary ?: return + val userLibrary = synchronized(this) { userLibrary ?: return } userLibrary.createPlaylist(name, songs) - for (listener in updateListeners) { - listener.onMusicChanges( - MusicRepository.Changes(deviceLibrary = false, userLibrary = true)) - } + notifyUserLibraryChange() } override suspend fun renamePlaylist(playlist: Playlist, name: String) { - val userLibrary = userLibrary ?: return + val userLibrary = synchronized(this) { userLibrary ?: return } userLibrary.renamePlaylist(playlist, name) - for (listener in updateListeners) { - listener.onMusicChanges( - MusicRepository.Changes(deviceLibrary = false, userLibrary = true)) - } + notifyUserLibraryChange() } override suspend fun deletePlaylist(playlist: Playlist) { - val userLibrary = userLibrary ?: return + val userLibrary = synchronized(this) { userLibrary ?: return } userLibrary.deletePlaylist(playlist) - for (listener in updateListeners) { - listener.onMusicChanges( - MusicRepository.Changes(deviceLibrary = false, userLibrary = true)) - } + notifyUserLibraryChange() } override suspend fun addToPlaylist(songs: List, playlist: Playlist) { - val userLibrary = userLibrary ?: return + val userLibrary = synchronized(this) { userLibrary ?: return } userLibrary.addToPlaylist(playlist, songs) - for (listener in updateListeners) { - listener.onMusicChanges( - MusicRepository.Changes(deviceLibrary = false, userLibrary = true)) - } + notifyUserLibraryChange() } override suspend fun rewritePlaylist(playlist: Playlist, songs: List) { - val userLibrary = userLibrary ?: return + val userLibrary = synchronized(this) { userLibrary ?: return } userLibrary.rewritePlaylist(playlist, songs) + notifyUserLibraryChange() + } + + @Synchronized + private fun notifyUserLibraryChange() { for (listener in updateListeners) { listener.onMusicChanges( MusicRepository.Changes(deviceLibrary = false, userLibrary = true)) } } + @Synchronized override fun requestIndex(withCache: Boolean) { indexingWorker?.requestIndex(withCache) } @@ -400,9 +395,10 @@ constructor( throw NoMusicException() } - // Successfully loaded the library, now save the cache and create the library in - // parallel. + // Successfully loaded the library, now save the cache, create the library, and + // read playlist information in parallel. logD("Discovered ${rawSongs.size} songs, starting finalization") + // TODO: Indicate playlist state in loading process? emitLoading(IndexingProgress.Indeterminate) val deviceLibraryChannel = Channel() val deviceLibraryJob =