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.

Author: Profpatsch

app/src/main/java/org/schabi/newpipe/player/ui/PlayerUiList.kt

@@ -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) {
-            if (playerUiType.isInstance(ui)) {
-                ui.destroyPlayer()
-                ui.destroy()
-                playerUis.remove(ui)
+        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()
         }
     }
 
@@ -67,18 +83,19 @@ 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) {
-            if (playerUiType.isInstance(ui)) {
-                when (val r = playerUiType.cast(ui)) {
-                    // try all UIs before returning null
-                    null -> continue
-                    else -> return r
+    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@runWithLockSync r
+                    }
                 }
             }
+            return@runWithLockSync null
         }
-        return null
-    }
 
     /**
      * @param playerUiType the class of the player UI to return;
@@ -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)
         }
     }

app/src/main/java/org/schabi/newpipe/util/GuardedByMutex.kt

@@ -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: T) {
+            setFun(t)
+            field = t
+        }
+}