music: fix reloads not cancelling prior ones

Caused by a dumb mistake in the cancellation code.
This commit is contained in:
Alexander Capehart 2024-01-01 14:36:26 -07:00
parent 28ff2b416a
commit 53870cd31b
No known key found for this signature in database
GPG key ID: 37DBE3621FE9AD47
2 changed files with 19 additions and 14 deletions

View file

@ -29,8 +29,10 @@ import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job import kotlinx.coroutines.Job
import kotlinx.coroutines.async import kotlinx.coroutines.async
import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.isActive
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext import kotlinx.coroutines.withContext
import kotlinx.coroutines.withTimeout
import kotlinx.coroutines.yield import kotlinx.coroutines.yield
import org.oxycblt.auxio.music.cache.CacheRepository import org.oxycblt.auxio.music.cache.CacheRepository
import org.oxycblt.auxio.music.device.DeviceLibrary import org.oxycblt.auxio.music.device.DeviceLibrary
@ -341,11 +343,11 @@ constructor(
} }
override fun index(worker: MusicRepository.IndexingWorker, withCache: Boolean) = override fun index(worker: MusicRepository.IndexingWorker, withCache: Boolean) =
worker.scope.launch { indexWrapper(worker, withCache) } worker.scope.launch { indexWrapper(worker.context, this, withCache) }
private suspend fun indexWrapper(worker: MusicRepository.IndexingWorker, withCache: Boolean) { private suspend fun indexWrapper(context: Context, scope: CoroutineScope, withCache: Boolean) {
try { try {
indexImpl(worker, withCache) indexImpl(context, scope, withCache)
} catch (e: CancellationException) { } catch (e: CancellationException) {
// Got cancelled, propagate upwards to top-level co-routine. // Got cancelled, propagate upwards to top-level co-routine.
logD("Loading routine was cancelled") logD("Loading routine was cancelled")
@ -359,13 +361,13 @@ constructor(
} }
} }
private suspend fun indexImpl(worker: MusicRepository.IndexingWorker, withCache: Boolean) { private suspend fun indexImpl(context: Context, scope: CoroutineScope, withCache: Boolean) {
// TODO: Find a way to break up this monster of a method, preferably as another class. // TODO: Find a way to break up this monster of a method, preferably as another class.
val start = System.currentTimeMillis() val start = System.currentTimeMillis()
// Make sure we have permissions before going forward. Theoretically this would be better // Make sure we have permissions before going forward. Theoretically this would be better
// done at the UI level, but that intertwines logic and display too much. // done at the UI level, but that intertwines logic and display too much.
if (ContextCompat.checkSelfPermission(worker.context, PERMISSION_READ_AUDIO) == if (ContextCompat.checkSelfPermission(context, PERMISSION_READ_AUDIO) ==
PackageManager.PERMISSION_DENIED) { PackageManager.PERMISSION_DENIED) {
logE("Permissions were not granted") logE("Permissions were not granted")
throw NoAudioPermissionException() throw NoAudioPermissionException()
@ -390,7 +392,7 @@ constructor(
emitIndexingProgress(IndexingProgress.Indeterminate) emitIndexingProgress(IndexingProgress.Indeterminate)
val mediaStoreQueryJob = val mediaStoreQueryJob =
worker.scope.async { scope.async {
val query = val query =
try { try {
mediaStoreExtractor.query(constraints) mediaStoreExtractor.query(constraints)
@ -428,7 +430,7 @@ constructor(
// does not exist. In the latter situation, it also applies it's own (inferior) metadata. // does not exist. In the latter situation, it also applies it's own (inferior) metadata.
logD("Starting MediaStore discovery") logD("Starting MediaStore discovery")
val mediaStoreJob = val mediaStoreJob =
worker.scope.async { scope.async {
try { try {
mediaStoreExtractor.consume(query, cache, incompleteSongs, completeSongs) mediaStoreExtractor.consume(query, cache, incompleteSongs, completeSongs)
} catch (e: Exception) { } catch (e: Exception) {
@ -446,7 +448,7 @@ constructor(
// metadata for them, and then forwards it to DeviceLibrary. // metadata for them, and then forwards it to DeviceLibrary.
logD("Starting tag extraction") logD("Starting tag extraction")
val tagJob = val tagJob =
worker.scope.async { scope.async {
try { try {
tagExtractor.consume(incompleteSongs, completeSongs) tagExtractor.consume(incompleteSongs, completeSongs)
} catch (e: Exception) { } catch (e: Exception) {
@ -461,7 +463,7 @@ constructor(
// and then forwards them to the primary loading loop. // and then forwards them to the primary loading loop.
logD("Starting DeviceLibrary creation") logD("Starting DeviceLibrary creation")
val deviceLibraryJob = val deviceLibraryJob =
worker.scope.async(Dispatchers.Default) { scope.async(Dispatchers.Default) {
val deviceLibrary = val deviceLibrary =
try { try {
deviceLibraryFactory.create( deviceLibraryFactory.create(
@ -488,6 +490,11 @@ constructor(
emitIndexingProgress(IndexingProgress.Songs(rawSongs.size, query.projectedTotal)) emitIndexingProgress(IndexingProgress.Songs(rawSongs.size, query.projectedTotal))
} }
withTimeout(10000) {
mediaStoreJob.await()
tagJob.await()
}
// Deliberately done after the involved initialization step to make it less likely // Deliberately done after the involved initialization step to make it less likely
// that the short-circuit occurs so quickly as to break the UI. // that the short-circuit occurs so quickly as to break the UI.
// TODO: Do not error, instead just wipe the entire library. // TODO: Do not error, instead just wipe the entire library.
@ -510,7 +517,7 @@ constructor(
// working on parent information. // working on parent information.
logD("Starting UserLibrary query") logD("Starting UserLibrary query")
val userLibraryQueryJob = val userLibraryQueryJob =
worker.scope.async { scope.async {
val rawPlaylists = val rawPlaylists =
try { try {
userLibraryFactory.query() userLibraryFactory.query()

View file

@ -32,7 +32,6 @@ import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job import kotlinx.coroutines.Job
import kotlinx.coroutines.launch
import org.oxycblt.auxio.BuildConfig import org.oxycblt.auxio.BuildConfig
import org.oxycblt.auxio.music.IndexingProgress import org.oxycblt.auxio.music.IndexingProgress
import org.oxycblt.auxio.music.IndexingState import org.oxycblt.auxio.music.IndexingState
@ -124,12 +123,11 @@ class IndexerService :
// --- CONTROLLER CALLBACKS --- // --- CONTROLLER CALLBACKS ---
override fun requestIndex(withCache: Boolean) { override fun requestIndex(withCache: Boolean) {
logD("Starting new indexing job") logD("Starting new indexing job (previous=${currentIndexJob?.hashCode()})")
// Cancel the previous music loading job. // Cancel the previous music loading job.
currentIndexJob?.cancel() currentIndexJob?.cancel()
// Start a new music loading job on a co-routine. // Start a new music loading job on a co-routine.
currentIndexJob = currentIndexJob = musicRepository.index(this@IndexerService, withCache)
indexScope.launch { musicRepository.index(this@IndexerService, withCache) }
} }
override val context = this override val context = this