all: use synchronized annotation

Use the @Synchronized annotation instead of synchronized.

Makes my ability to manage thread-safety on the shared objects much
easier. Because I don't have to think about what I should guard
and what I shouldn't.
This commit is contained in:
OxygenCobalt 2022-07-02 17:28:51 -06:00
parent 73e10c9519
commit d6f166a3ee
No known key found for this signature in database
GPG key ID: 37DBE3621FE9AD47
6 changed files with 161 additions and 178 deletions

View file

@ -49,10 +49,11 @@ class BitmapProvider(private val context: Context) {
* Load a bitmap from [song]. [target] should be a new object, not a reference to an existing * Load a bitmap from [song]. [target] should be a new object, not a reference to an existing
* callback. * callback.
*/ */
@Synchronized
fun load(song: Song, target: Target) { fun load(song: Song, target: Target) {
// Increment the generation value so that previous requests are invalidated. // Increment the generation value so that previous requests are invalidated.
// This is a second safeguard to mitigate instruction-by-instruction race conditions. // This is a second safeguard to mitigate instruction-by-instruction race conditions.
val generation = synchronized(this) { ++currentGeneration } val generation = ++currentGeneration
currentRequest?.run { disposable.dispose() } currentRequest?.run { disposable.dispose() }
currentRequest = null currentRequest = null

View file

@ -73,6 +73,7 @@ class Indexer {
get() = indexingState != null get() = indexingState != null
/** Register a [Controller] with this instance. */ /** Register a [Controller] with this instance. */
@Synchronized
fun registerController(controller: Controller) { fun registerController(controller: Controller) {
if (BuildConfig.DEBUG && this.controller != null) { if (BuildConfig.DEBUG && this.controller != null) {
logW("Controller is already registered") logW("Controller is already registered")
@ -83,15 +84,17 @@ class Indexer {
} }
/** Unregister a [Controller] with this instance. */ /** Unregister a [Controller] with this instance. */
@Synchronized
fun unregisterController(controller: Controller) { fun unregisterController(controller: Controller) {
if (BuildConfig.DEBUG && this.controller !== controller) { if (BuildConfig.DEBUG && this.controller !== controller) {
logW("Given controller did not match current controller") logW("Given controller did not match current controller")
return return
} }
synchronized(this) { this.controller = null } this.controller = null
} }
@Synchronized
fun registerCallback(callback: Callback) { fun registerCallback(callback: Callback) {
if (BuildConfig.DEBUG && this.callback != null) { if (BuildConfig.DEBUG && this.callback != null) {
logW("Callback is already registered") logW("Callback is already registered")
@ -106,6 +109,7 @@ class Indexer {
this.callback = callback this.callback = callback
} }
@Synchronized
fun unregisterCallback(callback: Callback) { fun unregisterCallback(callback: Callback) {
if (BuildConfig.DEBUG && this.callback !== callback) { if (BuildConfig.DEBUG && this.callback !== callback) {
logW("Given controller did not match current controller") logW("Given controller did not match current controller")
@ -115,8 +119,9 @@ class Indexer {
this.callback = null this.callback = null
} }
@Synchronized
fun index(context: Context) { fun index(context: Context) {
val generation = synchronized(this) { ++currentGeneration } val generation = ++currentGeneration
val notGranted = val notGranted =
ContextCompat.checkSelfPermission(context, Manifest.permission.READ_EXTERNAL_STORAGE) == ContextCompat.checkSelfPermission(context, Manifest.permission.READ_EXTERNAL_STORAGE) ==
@ -153,6 +158,7 @@ class Indexer {
* Request that re-indexing should be done. This should be used by components that do not manage * Request that re-indexing should be done. This should be used by components that do not manage
* the indexing process to re-index music. * the indexing process to re-index music.
*/ */
@Synchronized
fun requestReindex() { fun requestReindex() {
logD("Requesting reindex") logD("Requesting reindex")
controller?.onStartIndexing() controller?.onStartIndexing()
@ -164,46 +170,42 @@ class Indexer {
* canceled, in which it would be useful for any ongoing loading process to not accidentally * canceled, in which it would be useful for any ongoing loading process to not accidentally
* corrupt the current state. * corrupt the current state.
*/ */
@Synchronized
fun cancelLast() { fun cancelLast() {
logD("Cancelling last job") logD("Cancelling last job")
synchronized(this) { currentGeneration++
currentGeneration++ emitIndexing(null, currentGeneration)
emitIndexing(null, currentGeneration)
}
} }
@Synchronized
private fun emitIndexing(indexing: Indexing?, generation: Long) { private fun emitIndexing(indexing: Indexing?, generation: Long) {
synchronized(this) { if (currentGeneration != generation) {
if (currentGeneration != generation) { return
return
}
indexingState = indexing
// If we have canceled the loading process, we want to revert to a previous completion
// whenever possible to prevent state inconsistency.
val state =
indexingState?.let { State.Indexing(it) }
?: lastResponse?.let { State.Complete(it) }
controller?.onIndexerStateChanged(state)
callback?.onIndexerStateChanged(state)
} }
indexingState = indexing
// If we have canceled the loading process, we want to revert to a previous completion
// whenever possible to prevent state inconsistency.
val state =
indexingState?.let { State.Indexing(it) } ?: lastResponse?.let { State.Complete(it) }
controller?.onIndexerStateChanged(state)
callback?.onIndexerStateChanged(state)
} }
@Synchronized
private fun emitCompletion(response: Response, generation: Long) { private fun emitCompletion(response: Response, generation: Long) {
synchronized(this) { if (currentGeneration != generation) {
if (currentGeneration != generation) { return
return
}
lastResponse = response
indexingState = null
val state = State.Complete(response)
controller?.onIndexerStateChanged(state)
callback?.onIndexerStateChanged(state)
} }
lastResponse = response
indexingState = null
val state = State.Complete(response)
controller?.onIndexerStateChanged(state)
callback?.onIndexerStateChanged(state)
} }
/** /**

View file

@ -46,12 +46,12 @@ class MusicStore private constructor() {
callbacks.remove(callback) callbacks.remove(callback)
} }
/** Update the library in this instance. This is only meant for use by the internal indexer. */
@Synchronized
fun updateLibrary(newLibrary: Library?) { fun updateLibrary(newLibrary: Library?) {
synchronized(this) { library = newLibrary
library = newLibrary for (callback in callbacks) {
for (callback in callbacks) { callback.onLibraryChanged(library)
callback.onLibraryChanged(library)
}
} }
} }

View file

@ -101,6 +101,7 @@ class PlaybackStateDatabase(context: Context) :
// --- INTERFACE FUNCTIONS --- // --- INTERFACE FUNCTIONS ---
@Synchronized
fun read(library: MusicStore.Library): SavedState? { fun read(library: MusicStore.Library): SavedState? {
requireBackgroundThread() requireBackgroundThread()
@ -185,38 +186,37 @@ class PlaybackStateDatabase(context: Context) :
} }
/** Clear the previously written [SavedState] and write a new one. */ /** Clear the previously written [SavedState] and write a new one. */
@Synchronized
fun write(state: SavedState) { fun write(state: SavedState) {
requireBackgroundThread() requireBackgroundThread()
synchronized(this) { val song = state.queue.getOrNull(state.index)
val song = state.queue.getOrNull(state.index)
if (song != null) { if (song != null) {
val rawState = val rawState =
RawState( RawState(
index = state.index, index = state.index,
positionMs = state.positionMs, positionMs = state.positionMs,
repeatMode = state.repeatMode, repeatMode = state.repeatMode,
isShuffled = state.isShuffled, isShuffled = state.isShuffled,
songId = song.id, songId = song.id,
parentId = state.parent?.id, parentId = state.parent?.id,
playbackMode = playbackMode =
when (state.parent) { when (state.parent) {
null -> PlaybackMode.ALL_SONGS null -> PlaybackMode.ALL_SONGS
is Album -> PlaybackMode.IN_ALBUM is Album -> PlaybackMode.IN_ALBUM
is Artist -> PlaybackMode.IN_ARTIST is Artist -> PlaybackMode.IN_ARTIST
is Genre -> PlaybackMode.IN_GENRE is Genre -> PlaybackMode.IN_GENRE
}) })
writeRawState(rawState) writeRawState(rawState)
writeQueue(state.queue) writeQueue(state.queue)
} else { } else {
writeRawState(null) writeRawState(null)
writeQueue(null) writeQueue(null)
}
logD("Wrote state to database")
} }
logD("Wrote state to database")
} }
private fun writeRawState(rawState: RawState?) { private fun writeRawState(rawState: RawState?) {

View file

@ -45,8 +45,6 @@ import org.oxycblt.auxio.util.logW
* *
* All access should be done with [PlaybackStateManager.getInstance]. * All access should be done with [PlaybackStateManager.getInstance].
* @author OxygenCobalt * @author OxygenCobalt
*
* TODO: Replace synchronized calls with annotation
*/ */
class PlaybackStateManager private constructor() { class PlaybackStateManager private constructor() {
private val musicStore = MusicStore.getInstance() private val musicStore = MusicStore.getInstance()
@ -93,45 +91,45 @@ class PlaybackStateManager private constructor() {
private var controller: Controller? = null private var controller: Controller? = null
/** Add a callback to this instance. Make sure to remove it when done. */ /** Add a callback to this instance. Make sure to remove it when done. */
@Synchronized
fun addCallback(callback: Callback) { fun addCallback(callback: Callback) {
synchronized(this) { if (isInitialized) {
if (isInitialized) { callback.onNewPlayback(index, queue, parent)
callback.onNewPlayback(index, queue, parent) callback.onPositionChanged(positionMs)
callback.onPositionChanged(positionMs) callback.onPlayingChanged(isPlaying)
callback.onPlayingChanged(isPlaying) callback.onRepeatChanged(repeatMode)
callback.onRepeatChanged(repeatMode) callback.onShuffledChanged(isShuffled)
callback.onShuffledChanged(isShuffled)
}
callbacks.add(callback)
} }
callbacks.add(callback)
} }
/** Remove a [Callback] bound to this instance. */ /** Remove a [Callback] bound to this instance. */
@Synchronized
fun removeCallback(callback: Callback) { fun removeCallback(callback: Callback) {
callbacks.remove(callback) callbacks.remove(callback)
} }
/** Register a [Controller] with this instance. */ /** Register a [Controller] with this instance. */
@Synchronized
fun registerController(controller: Controller) { fun registerController(controller: Controller) {
if (BuildConfig.DEBUG && this.controller != null) { if (BuildConfig.DEBUG && this.controller != null) {
logW("Controller is already registered") logW("Controller is already registered")
return return
} }
synchronized(this) { if (isInitialized) {
if (isInitialized) { controller.loadSong(song)
controller.loadSong(song) controller.seekTo(positionMs)
controller.seekTo(positionMs) controller.onPlayingChanged(isPlaying)
controller.onPlayingChanged(isPlaying) controller.onPlayingChanged(isPlaying)
controller.onPlayingChanged(isPlaying)
}
this.controller = controller
} }
this.controller = controller
} }
/** Unregister a [Controller] with this instance. */ /** Unregister a [Controller] with this instance. */
@Synchronized
fun unregisterController(controller: Controller) { fun unregisterController(controller: Controller) {
if (BuildConfig.DEBUG && this.controller !== controller) { if (BuildConfig.DEBUG && this.controller !== controller) {
logW("Given controller did not match current controller") logW("Given controller did not match current controller")
@ -147,84 +145,78 @@ class PlaybackStateManager private constructor() {
* Play a [song]. * Play a [song].
* @param playbackMode The [PlaybackMode] to construct the queue off of. * @param playbackMode The [PlaybackMode] to construct the queue off of.
*/ */
@Synchronized
fun play(song: Song, playbackMode: PlaybackMode, settings: Settings) { fun play(song: Song, playbackMode: PlaybackMode, settings: Settings) {
val library = musicStore.library ?: return val library = musicStore.library ?: return
synchronized(this) { parent =
parent = when (playbackMode) {
when (playbackMode) { PlaybackMode.ALL_SONGS -> null
PlaybackMode.ALL_SONGS -> null PlaybackMode.IN_ALBUM -> song.album
PlaybackMode.IN_ALBUM -> song.album PlaybackMode.IN_ARTIST -> song.album.artist
PlaybackMode.IN_ARTIST -> song.album.artist PlaybackMode.IN_GENRE -> song.genre
PlaybackMode.IN_GENRE -> song.genre }
}
applyNewQueue(library, settings, settings.keepShuffle && isShuffled, song) applyNewQueue(library, settings, settings.keepShuffle && isShuffled, song)
seekTo(0) seekTo(0)
notifyNewPlayback() notifyNewPlayback()
notifyShuffledChanged() notifyShuffledChanged()
isPlaying = true isPlaying = true
isInitialized = true isInitialized = true
}
} }
/** /**
* Play a [parent], such as an artist or album. * Play a [parent], such as an artist or album.
* @param shuffled Whether the queue is shuffled or not * @param shuffled Whether the queue is shuffled or not
*/ */
@Synchronized
fun play(parent: MusicParent, shuffled: Boolean, settings: Settings) { fun play(parent: MusicParent, shuffled: Boolean, settings: Settings) {
val library = musicStore.library ?: return val library = musicStore.library ?: return
this.parent = parent
synchronized(this) { applyNewQueue(library, settings, shuffled, null)
this.parent = parent seekTo(0)
applyNewQueue(library, settings, shuffled, null) notifyNewPlayback()
seekTo(0) notifyShuffledChanged()
notifyNewPlayback() isPlaying = true
notifyShuffledChanged() isInitialized = true
isPlaying = true
isInitialized = true
}
} }
/** Shuffle all songs. */ /** Shuffle all songs. */
@Synchronized
fun shuffleAll(settings: Settings) { fun shuffleAll(settings: Settings) {
val library = musicStore.library ?: return val library = musicStore.library ?: return
synchronized(this) { parent = null
parent = null applyNewQueue(library, settings, true, null)
applyNewQueue(library, settings, true, null) seekTo(0)
seekTo(0) notifyNewPlayback()
notifyNewPlayback() notifyShuffledChanged()
notifyShuffledChanged() isPlaying = true
isPlaying = true isInitialized = true
isInitialized = true
}
} }
// --- QUEUE FUNCTIONS --- // --- QUEUE FUNCTIONS ---
/** Go to the next song, along with doing all the checks that entails. */ /** Go to the next song, along with doing all the checks that entails. */
@Synchronized
fun next() { fun next() {
synchronized(this) { // Increment the index, if it cannot be incremented any further, then
// Increment the index, if it cannot be incremented any further, then // repeat and pause/resume playback depending on the setting
// repeat and pause/resume playback depending on the setting if (index < _queue.lastIndex) {
if (index < _queue.lastIndex) { goto(index + 1, true)
goto(index + 1, true) } else {
} else { goto(0, repeatMode == RepeatMode.ALL)
goto(0, repeatMode == RepeatMode.ALL)
}
} }
} }
/** Go to the previous song, doing any checks that are needed. */ /** Go to the previous song, doing any checks that are needed. */
@Synchronized
fun prev() { fun prev() {
synchronized(this) { // If enabled, rewind before skipping back if the position is past 3 seconds [3000ms]
// If enabled, rewind before skipping back if the position is past 3 seconds [3000ms] if (controller?.shouldPrevRewind() == true) {
if (controller?.shouldPrevRewind() == true) { rewind()
rewind() isPlaying = true
isPlaying = true } else {
} else { goto(max(index - 1, 0), true)
goto(max(index - 1, 0), true)
}
} }
} }
@ -236,66 +228,57 @@ class PlaybackStateManager private constructor() {
} }
/** Add a [song] to the top of the queue. */ /** Add a [song] to the top of the queue. */
@Synchronized
fun playNext(song: Song) { fun playNext(song: Song) {
synchronized(this) { _queue.add(index + 1, song)
_queue.add(index + 1, song) notifyQueueChanged()
notifyQueueChanged()
}
} }
/** Add a list of [songs] to the top of the queue. */ /** Add a list of [songs] to the top of the queue. */
@Synchronized
fun playNext(songs: List<Song>) { fun playNext(songs: List<Song>) {
synchronized(this) { _queue.addAll(index + 1, songs)
_queue.addAll(index + 1, songs) notifyQueueChanged()
notifyQueueChanged()
}
} }
/** Add a [song] to the end of the queue. */ /** Add a [song] to the end of the queue. */
@Synchronized
fun addToQueue(song: Song) { fun addToQueue(song: Song) {
synchronized(this) { _queue.add(song)
_queue.add(song) notifyQueueChanged()
notifyQueueChanged()
}
} }
/** Add a list of [songs] to the end of the queue. */ /** Add a list of [songs] to the end of the queue. */
@Synchronized
fun addToQueue(songs: List<Song>) { fun addToQueue(songs: List<Song>) {
synchronized(this) { _queue.addAll(songs)
_queue.addAll(songs) notifyQueueChanged()
notifyQueueChanged()
}
} }
/** Move a queue item at [from] to a position at [to]. Will ignore invalid indexes. */ /** Move a queue item at [from] to a position at [to]. Will ignore invalid indexes. */
@Synchronized
fun moveQueueItem(from: Int, to: Int) { fun moveQueueItem(from: Int, to: Int) {
logD("Moving item $from to position $to") logD("Moving item $from to position $to")
_queue.add(to, _queue.removeAt(from))
synchronized(this) { notifyQueueChanged()
_queue.add(to, _queue.removeAt(from))
notifyQueueChanged()
}
} }
/** Remove a queue item at [index]. Will ignore invalid indexes. */ /** Remove a queue item at [index]. Will ignore invalid indexes. */
@Synchronized
fun removeQueueItem(index: Int) { fun removeQueueItem(index: Int) {
logD("Removing item ${_queue[index].rawName}") logD("Removing item ${_queue[index].rawName}")
_queue.removeAt(index)
synchronized(this) { notifyQueueChanged()
_queue.removeAt(index)
notifyQueueChanged()
}
} }
/** Set whether this instance is [shuffled]. Updates the queue accordingly. */ /** Set whether this instance is [shuffled]. Updates the queue accordingly. */
@Synchronized
fun reshuffle(shuffled: Boolean, settings: Settings) { fun reshuffle(shuffled: Boolean, settings: Settings) {
val library = musicStore.library ?: return val library = musicStore.library ?: return
synchronized(this) { val song = song ?: return
val song = song ?: return applyNewQueue(library, settings, shuffled, song)
applyNewQueue(library, settings, shuffled, song) notifyQueueChanged()
notifyQueueChanged() notifyShuffledChanged()
notifyShuffledChanged()
}
} }
private fun applyNewQueue( private fun applyNewQueue(
@ -343,19 +326,18 @@ class PlaybackStateManager private constructor() {
* @param positionMs The new position in millis. * @param positionMs The new position in millis.
* @see seekTo * @see seekTo
*/ */
@Synchronized
fun synchronizePosition(controller: Controller, positionMs: Long) { fun synchronizePosition(controller: Controller, positionMs: Long) {
if (BuildConfig.DEBUG && this.controller !== controller) { if (BuildConfig.DEBUG && this.controller !== controller) {
logW("Given controller did not match current controller") logW("Given controller did not match current controller")
return return
} }
synchronized(this) { // Don't accept any bugged positions that are over the duration of the song.
// Don't accept any bugged positions that are over the duration of the song. val maxDuration = song?.durationMs ?: -1
val maxDuration = song?.durationMs ?: -1 if (positionMs <= maxDuration) {
if (positionMs <= maxDuration) { this.positionMs = positionMs
this.positionMs = positionMs notifyPositionChanged()
notifyPositionChanged()
}
} }
} }
@ -410,8 +392,8 @@ class PlaybackStateManager private constructor() {
): PlaybackStateDatabase.SavedState? { ): PlaybackStateDatabase.SavedState? {
logD("Getting state from DB") logD("Getting state from DB")
val start: Long = System.currentTimeMillis() val start = System.currentTimeMillis()
val state: PlaybackStateDatabase.SavedState? = database.read(library) val state = database.read(library)
logD("State read completed successfully in ${System.currentTimeMillis() - start}ms") logD("State read completed successfully in ${System.currentTimeMillis() - start}ms")

View file

@ -40,7 +40,6 @@ import androidx.annotation.Px
import androidx.annotation.StringRes import androidx.annotation.StringRes
import androidx.core.content.ContextCompat import androidx.core.content.ContextCompat
import kotlin.reflect.KClass import kotlin.reflect.KClass
import kotlin.system.exitProcess
import org.oxycblt.auxio.IntegerTable import org.oxycblt.auxio.IntegerTable
import org.oxycblt.auxio.MainActivity import org.oxycblt.auxio.MainActivity
@ -236,4 +235,3 @@ fun Context.newBroadcastPendingIntent(what: String): PendingIntent =
IntegerTable.REQUEST_CODE, IntegerTable.REQUEST_CODE,
Intent(what).setFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY), Intent(what).setFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY),
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) PendingIntent.FLAG_IMMUTABLE else 0) if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) PendingIntent.FLAG_IMMUTABLE else 0)