mirror of
https://github.com/TeamNewPipe/NewPipe
synced 2025-01-04 22:40:32 +00:00
PlayerUiList: guard list actions with mutex
The new implementation would throw `ConcurrentModificationExceptions` when destroying the UIs. So let’s play it safe and put the list behind a mutex. Adds a helper class `GuardedByMutex` that can be wrapped around a property to force all use-sites to acquire the lock before doing anything with the data.
This commit is contained in:
parent
5b92de4f76
commit
cbade0b54d
@ -1,10 +1,10 @@
|
||||
package org.schabi.newpipe.player.ui
|
||||
|
||||
import androidx.core.util.Consumer
|
||||
import org.schabi.newpipe.util.GuardedByMutex
|
||||
import java.util.Optional
|
||||
|
||||
class PlayerUiList(vararg initialPlayerUis: PlayerUi) {
|
||||
val playerUis = mutableListOf<PlayerUi>()
|
||||
var playerUis = GuardedByMutex(mutableListOf<PlayerUi>())
|
||||
|
||||
/**
|
||||
* Creates a [PlayerUiList] starting with the provided player uis. The provided player uis
|
||||
@ -16,7 +16,9 @@ class PlayerUiList(vararg initialPlayerUis: PlayerUi) {
|
||||
* @param initialPlayerUis the player uis this list should start with; the order will be kept
|
||||
*/
|
||||
init {
|
||||
playerUis.addAll(listOf(*initialPlayerUis))
|
||||
playerUis.runWithLockSync {
|
||||
lockData.addAll(listOf(*initialPlayerUis))
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@ -41,7 +43,9 @@ class PlayerUiList(vararg initialPlayerUis: PlayerUi) {
|
||||
}
|
||||
}
|
||||
|
||||
playerUis.add(playerUi)
|
||||
playerUis.runWithLockSync {
|
||||
lockData.add(playerUi)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@ -52,12 +56,24 @@ class PlayerUiList(vararg initialPlayerUis: PlayerUi) {
|
||||
* @param T the class type parameter </T>
|
||||
* */
|
||||
fun <T> destroyAll(playerUiType: Class<T?>) {
|
||||
for (ui in playerUis) {
|
||||
val toDestroy = mutableListOf<PlayerUi>()
|
||||
|
||||
// short blocking removal from class to prevent interfering from other threads
|
||||
playerUis.runWithLockSync {
|
||||
val new = mutableListOf<PlayerUi>()
|
||||
for (ui in lockData) {
|
||||
if (playerUiType.isInstance(ui)) {
|
||||
toDestroy.add(ui)
|
||||
} else {
|
||||
new.add(ui)
|
||||
}
|
||||
}
|
||||
lockData = new
|
||||
}
|
||||
// then actually destroy the UIs
|
||||
for (ui in toDestroy) {
|
||||
ui.destroyPlayer()
|
||||
ui.destroy()
|
||||
playerUis.remove(ui)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -67,17 +83,18 @@ class PlayerUiList(vararg initialPlayerUis: PlayerUi) {
|
||||
* @param T the class type parameter
|
||||
* @return the first player UI of the required type found in the list, or null
|
||||
</T> */
|
||||
fun <T> get(playerUiType: Class<T>): T? {
|
||||
for (ui in playerUis) {
|
||||
fun <T> get(playerUiType: Class<T>): T? =
|
||||
playerUis.runWithLockSync {
|
||||
for (ui in lockData) {
|
||||
if (playerUiType.isInstance(ui)) {
|
||||
when (val r = playerUiType.cast(ui)) {
|
||||
// try all UIs before returning null
|
||||
null -> continue
|
||||
else -> return r
|
||||
else -> return@runWithLockSync r
|
||||
}
|
||||
}
|
||||
}
|
||||
return null
|
||||
return@runWithLockSync null
|
||||
}
|
||||
|
||||
/**
|
||||
@ -96,7 +113,11 @@ class PlayerUiList(vararg initialPlayerUis: PlayerUi) {
|
||||
* @param consumer the consumer to call with player UIs
|
||||
*/
|
||||
fun call(consumer: java.util.function.Consumer<PlayerUi>) {
|
||||
for (ui in playerUis) {
|
||||
// copy the list out of the mutex before calling the consumer which might block
|
||||
val new = playerUis.runWithLockSync {
|
||||
lockData.toMutableList()
|
||||
}
|
||||
for (ui in new) {
|
||||
consumer.accept(ui)
|
||||
}
|
||||
}
|
||||
|
47
app/src/main/java/org/schabi/newpipe/util/GuardedByMutex.kt
Normal file
47
app/src/main/java/org/schabi/newpipe/util/GuardedByMutex.kt
Normal file
@ -0,0 +1,47 @@
|
||||
package org.schabi.newpipe.util
|
||||
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import kotlinx.coroutines.sync.Mutex
|
||||
import kotlinx.coroutines.sync.withLock
|
||||
|
||||
/** Guard the given data so that it can only be accessed by locking the mutex first.
|
||||
*
|
||||
* Inspired by [this blog post](https://jonnyzzz.com/blog/2017/03/01/guarded-by-lock/)
|
||||
* */
|
||||
class GuardedByMutex<T>(
|
||||
private var data: T,
|
||||
private val lock: Mutex = Mutex(locked = false),
|
||||
) {
|
||||
|
||||
/** Lock the mutex and access the data, blocking the current thread.
|
||||
* @param action to run with locked mutex
|
||||
* */
|
||||
fun <Y> runWithLockSync(
|
||||
action: MutexData<T>.() -> Y
|
||||
) =
|
||||
runBlocking {
|
||||
lock.withLock {
|
||||
MutexData(data, { d -> data = d }).action()
|
||||
}
|
||||
}
|
||||
|
||||
/** Lock the mutex and access the data, suspending the coroutine.
|
||||
* @param action to run with locked mutex
|
||||
* */
|
||||
suspend fun <Y> runWithLock(action: MutexData<T>.() -> Y) =
|
||||
lock.withLock {
|
||||
MutexData(data, { d -> data = d }).action()
|
||||
}
|
||||
}
|
||||
|
||||
/** The data inside a [GuardedByMutex], which can be accessed via [lockData].
|
||||
* [lockData] is a `var`, so you can `set` it as well.
|
||||
* */
|
||||
class MutexData<T>(data: T, val setFun: (T) -> Unit) {
|
||||
/** The data inside this [GuardedByMutex] */
|
||||
var lockData: T = data
|
||||
set(t) {
|
||||
setFun(t)
|
||||
field = t
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue
Block a user