util: add guard abstraction

Add an abstraction over the generation system called GenerationGuard.

This allows cheap cooperative threading to be implemented consistently
in many places.
This commit is contained in:
OxygenCobalt 2022-07-09 11:15:52 -06:00
parent f9a61c8ef7
commit a63b3791d2
No known key found for this signature in database
GPG key ID: 37DBE3621FE9AD47
6 changed files with 61 additions and 43 deletions

View file

@ -39,6 +39,7 @@ import org.oxycblt.auxio.settings.Settings
import org.oxycblt.auxio.ui.Sort import org.oxycblt.auxio.ui.Sort
import org.oxycblt.auxio.ui.recycler.Header import org.oxycblt.auxio.ui.recycler.Header
import org.oxycblt.auxio.ui.recycler.Item import org.oxycblt.auxio.ui.recycler.Item
import org.oxycblt.auxio.util.GenerationGuard
import org.oxycblt.auxio.util.application import org.oxycblt.auxio.util.application
import org.oxycblt.auxio.util.logD import org.oxycblt.auxio.util.logD
import org.oxycblt.auxio.util.logW import org.oxycblt.auxio.util.logW
@ -112,6 +113,8 @@ class DetailViewModel(application: Application) :
currentGenre.value?.let(::refreshGenreData) currentGenre.value?.let(::refreshGenreData)
} }
private val songGuard = GenerationGuard()
fun setSongId(id: Long) { fun setSongId(id: Long) {
if (_currentSong.value?.run { song.id } == id) return if (_currentSong.value?.run { song.id } == id) return
val library = unlikelyToBeNull(musicStore.library) val library = unlikelyToBeNull(musicStore.library)
@ -158,16 +161,12 @@ class DetailViewModel(application: Application) :
private fun generateDetailSong(song: Song) { private fun generateDetailSong(song: Song) {
_currentSong.value = DetailSong(song, null) _currentSong.value = DetailSong(song, null)
viewModelScope.launch(Dispatchers.IO) { viewModelScope.launch(Dispatchers.IO) {
val generation = songGuard.newHandle()
val info = generateDetailSongInfo(song) val info = generateDetailSongInfo(song)
songGuard.yield(generation)
// Theoretically, the song could have been changed again while we were
// extracting song information, so make sure that we can update the song
// in the first place.
if (_currentSong.value?.run { this.song.id } == song.id) {
_currentSong.value = DetailSong(song, info) _currentSong.value = DetailSong(song, info)
} }
} }
}
private fun generateDetailSongInfo(song: Song): SongInfo { private fun generateDetailSongInfo(song: Song): SongInfo {
val extractor = MediaExtractor() val extractor = MediaExtractor()

View file

@ -53,7 +53,7 @@ import org.oxycblt.auxio.util.logW
* The base implementation for all image fetchers in Auxio. * The base implementation for all image fetchers in Auxio.
* @author OxygenCobalt * @author OxygenCobalt
* *
* TODO: Artist images * TODO: File-system derived images [cover.jpg, Artist Images]
*/ */
abstract class BaseFetcher : Fetcher { abstract class BaseFetcher : Fetcher {
/** /**

View file

@ -25,6 +25,7 @@ import coil.request.Disposable
import coil.request.ImageRequest import coil.request.ImageRequest
import coil.size.Size import coil.size.Size
import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.music.Song
import org.oxycblt.auxio.util.GenerationGuard
/** /**
* A utility to provide bitmaps in a manner less prone to race conditions. * A utility to provide bitmaps in a manner less prone to race conditions.
@ -39,7 +40,7 @@ import org.oxycblt.auxio.music.Song
*/ */
class BitmapProvider(private val context: Context) { class BitmapProvider(private val context: Context) {
private var currentRequest: Request? = null private var currentRequest: Request? = null
private var currentGeneration = 0L private var guard = GenerationGuard()
/** If this provider is currently attempting to load something. */ /** If this provider is currently attempting to load something. */
val isBusy: Boolean val isBusy: Boolean
@ -51,9 +52,7 @@ class BitmapProvider(private val context: Context) {
*/ */
@Synchronized @Synchronized
fun load(song: Song, target: Target) { fun load(song: Song, target: Target) {
// Increment the generation value so that previous requests are invalidated. val generation = guard.newHandle()
// This is a second safeguard to mitigate instruction-by-instruction race conditions.
val generation = ++currentGeneration
currentRequest?.run { disposable.dispose() } currentRequest?.run { disposable.dispose() }
currentRequest = null currentRequest = null
@ -65,12 +64,12 @@ class BitmapProvider(private val context: Context) {
.size(Size.ORIGINAL) .size(Size.ORIGINAL)
.target( .target(
onSuccess = { onSuccess = {
if (generation == currentGeneration) { if (guard.check(generation)) {
target.onCompleted(it.toBitmap()) target.onCompleted(it.toBitmap())
} }
}, },
onError = { onError = {
if (generation == currentGeneration) { if (guard.check(generation)) {
target.onCompleted(null) target.onCompleted(null)
} }
}) })

View file

@ -23,7 +23,6 @@ import android.content.pm.PackageManager
import android.database.Cursor import android.database.Cursor
import android.os.Build import android.os.Build
import androidx.core.content.ContextCompat import androidx.core.content.ContextCompat
import kotlin.coroutines.cancellation.CancellationException
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext import kotlinx.coroutines.withContext
import org.oxycblt.auxio.BuildConfig import org.oxycblt.auxio.BuildConfig
@ -34,6 +33,7 @@ import org.oxycblt.auxio.music.MusicStore
import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.music.Song
import org.oxycblt.auxio.settings.Settings import org.oxycblt.auxio.settings.Settings
import org.oxycblt.auxio.ui.Sort import org.oxycblt.auxio.ui.Sort
import org.oxycblt.auxio.util.GenerationGuard
import org.oxycblt.auxio.util.logD import org.oxycblt.auxio.util.logD
import org.oxycblt.auxio.util.logE import org.oxycblt.auxio.util.logE
import org.oxycblt.auxio.util.logW import org.oxycblt.auxio.util.logW
@ -64,7 +64,7 @@ class Indexer {
private var lastResponse: Response? = null private var lastResponse: Response? = null
private var indexingState: Indexing? = null private var indexingState: Indexing? = null
private var currentGeneration = 0L private var guard = GenerationGuard()
private var controller: Controller? = null private var controller: Controller? = null
private var callback: Callback? = null private var callback: Callback? = null
@ -133,7 +133,7 @@ class Indexer {
suspend fun index(context: Context) { suspend fun index(context: Context) {
requireBackgroundThread() requireBackgroundThread()
val generation = synchronized(this) { ++currentGeneration } val generation = guard.newHandle()
val notGranted = val notGranted =
ContextCompat.checkSelfPermission(context, Manifest.permission.READ_EXTERNAL_STORAGE) == ContextCompat.checkSelfPermission(context, Manifest.permission.READ_EXTERNAL_STORAGE) ==
@ -184,13 +184,13 @@ class Indexer {
@Synchronized @Synchronized
fun cancelLast() { fun cancelLast() {
logD("Cancelling last job") logD("Cancelling last job")
currentGeneration++ val generation = guard.newHandle()
emitIndexing(null, currentGeneration) emitIndexing(null, generation)
} }
@Synchronized @Synchronized
private fun emitIndexing(indexing: Indexing?, generation: Long) { private fun emitIndexing(indexing: Indexing?, generation: Long) {
checkGeneration(generation) guard.yield(generation)
if (indexing == indexingState) { if (indexing == indexingState) {
// Ignore redundant states used when the backends just want to check for // Ignore redundant states used when the backends just want to check for
@ -210,32 +210,23 @@ class Indexer {
} }
private suspend fun emitCompletion(response: Response, generation: Long) { private suspend fun emitCompletion(response: Response, generation: Long) {
synchronized(this) { guard.yield(generation)
checkGeneration(generation)
// Do not check for redundancy here, as we actually need to notify a switch
// from Indexing -> Complete and not Indexing -> Indexing or Complete -> Complete.
lastResponse = response
indexingState = null
}
val state = State.Complete(response)
// Swap to the Main thread so that downstream callbacks don't crash from being on // Swap to the Main thread so that downstream callbacks don't crash from being on
// a background thread. Does not occur in emitIndexing due to efficiency reasons. // a background thread. Does not occur in emitIndexing due to efficiency reasons.
withContext(Dispatchers.Main) { withContext(Dispatchers.Main) {
synchronized(this) {
// Do not check for redundancy here, as we actually need to notify a switch
// from Indexing -> Complete and not Indexing -> Indexing or Complete -> Complete.
lastResponse = response
indexingState = null
val state = State.Complete(response)
controller?.onIndexerStateChanged(state) controller?.onIndexerStateChanged(state)
callback?.onIndexerStateChanged(state) callback?.onIndexerStateChanged(state)
} }
} }
private fun checkGeneration(generation: Long) {
if (currentGeneration != generation) {
// Not the running task anymore, cancel this co-routine. This allows a yield-like
// behavior to be implemented in a far cheaper manner for each backend.
throw CancellationException()
}
} }
/** /**

View file

@ -471,7 +471,6 @@ class Api21MediaStoreBackend : MediaStoreBackend() {
open class BaseApi29MediaStoreBackend : MediaStoreBackend() { open class BaseApi29MediaStoreBackend : MediaStoreBackend() {
private var volumeIndex = -1 private var volumeIndex = -1
private var relativePathIndex = -1 private var relativePathIndex = -1
private var dateTakenIndex = -1
override val projection: Array<String> override val projection: Array<String>
get() = get() =
@ -499,7 +498,6 @@ open class BaseApi29MediaStoreBackend : MediaStoreBackend() {
volumeIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.VOLUME_NAME) volumeIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.VOLUME_NAME)
relativePathIndex = relativePathIndex =
cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.RELATIVE_PATH) cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.RELATIVE_PATH)
dateTakenIndex = cursor.getColumnIndexOrThrow(MediaStore.Audio.AudioColumns.DATE_TAKEN)
} }
val volumeName = cursor.getString(volumeIndex) val volumeName = cursor.getString(volumeIndex)
@ -512,7 +510,6 @@ open class BaseApi29MediaStoreBackend : MediaStoreBackend() {
audio.dir = Directory(volume, relativePath.removeSuffix(File.separator)) audio.dir = Directory(volume, relativePath.removeSuffix(File.separator))
} }
return audio return audio
} }
} }

View file

@ -22,6 +22,7 @@ import android.text.format.DateUtils
import androidx.core.math.MathUtils import androidx.core.math.MathUtils
import java.lang.reflect.Field import java.lang.reflect.Field
import java.lang.reflect.Method import java.lang.reflect.Method
import java.util.concurrent.CancellationException
import kotlin.reflect.KClass import kotlin.reflect.KClass
import org.oxycblt.auxio.BuildConfig import org.oxycblt.auxio.BuildConfig
@ -77,3 +78,34 @@ fun lazyReflectedField(clazz: KClass<*>, field: String) = lazy {
fun lazyReflectedMethod(clazz: KClass<*>, method: String) = lazy { fun lazyReflectedMethod(clazz: KClass<*>, method: String) = lazy {
clazz.java.getDeclaredMethod(method).also { it.isAccessible = true } clazz.java.getDeclaredMethod(method).also { it.isAccessible = true }
} }
/**
* A generation-based abstraction that allows cheap cooperative multi-threading in shared object
* contexts. Every new task should call [newHandle], while every running task should call [check] or
* [yield] depending on the context.
*
* @author OxygenCobalt
*/
class GenerationGuard {
private var currentHandle = 0L
/**
* Returns a new handle to the calling task while invalidating the generations of the previous
* task.
*/
@Synchronized fun newHandle() = ++currentHandle
/** Check if the given [handle] is still the one represented by this class. */
@Synchronized fun check(handle: Long) = handle == currentHandle
/**
* Alternative to [kotlinx.coroutines.yield], that achieves the same behavior but in a much
* cheaper manner.
*/
@Synchronized
fun yield(handle: Long) {
if (!check(handle)) {
throw CancellationException()
}
}
}