mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-10 00:16:59 +00:00
feat(playback): add SessionRegistry owning session reachability
test(playback): pin SessionRegistry's identity-vs-equality drop guard
This commit is contained in:
davotoula
+140
@@ -0,0 +1,140 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.service.playback.playerPool
|
||||
|
||||
import androidx.collection.LruCache
|
||||
|
||||
/**
|
||||
* Owns which sessions are reachable, and is the only place that decides one has been dropped.
|
||||
*
|
||||
* Two tiers: an LRU of idle entries and a map of ids that are currently playing. A playing entry
|
||||
* stays in [idle] as well — [playing] only makes an eviction non-dropping. That is deliberate: it
|
||||
* matches today's behaviour exactly (a playing session keeps consuming a cache slot until it ages
|
||||
* out), and it is what keeps the replacement guard in [entryRemoved] load-bearing rather than dead
|
||||
* code.
|
||||
*
|
||||
* Generic over the entry type so the bookkeeping — the part that has actually been wrong — is unit
|
||||
* testable on the JVM with no Android or media3 objects involved.
|
||||
*
|
||||
* Uses `androidx.collection.LruCache` rather than `android.util.LruCache`: the latter is stubbed
|
||||
* out under `unitTests.isReturnDefaultValues = true`, which would make every test here vacuous.
|
||||
* The contract is otherwise identical, including `entryRemoved` firing outside the lock.
|
||||
*
|
||||
* Not thread safe. Every caller runs on the main thread (see MediaSessionPool).
|
||||
*/
|
||||
internal class SessionRegistry<T : Any>(
|
||||
maxIdle: Int,
|
||||
private val onDropped: (T) -> Unit,
|
||||
) {
|
||||
private val playing = mutableMapOf<String, T>()
|
||||
|
||||
// Set while an explicit drop is tearing an entry out of [idle], so the resulting entryRemoved
|
||||
// callback doesn't signal a second time for the same drop.
|
||||
//
|
||||
// Resetting to false in `finally` (rather than restoring a saved value) is sound because the
|
||||
// guarded region cannot nest: onDropped is never invoked while the flag is set, so nothing
|
||||
// inside it can re-enter drop()/dropAll(). That non-nesting property is the invariant — not
|
||||
// single-threadedness — and a future edit that called onDropped inside the guarded region
|
||||
// would break it.
|
||||
private var suppressDropSignal = false
|
||||
|
||||
private val idle =
|
||||
object : LruCache<String, T>(maxIdle) {
|
||||
override fun entryRemoved(
|
||||
evicted: Boolean,
|
||||
key: String,
|
||||
oldValue: T,
|
||||
newValue: T?,
|
||||
) {
|
||||
// A replacement is not a drop: re-putting the same entry (pause -> idle) fires this
|
||||
// with newValue === oldValue, and retiring then would kill the session we are
|
||||
// keeping. `newValue == null` needs no separate test — V is non-null by type, so a
|
||||
// null newValue can never be identical to oldValue.
|
||||
if (suppressDropSignal) return
|
||||
if (newValue !== oldValue && !playing.containsKey(key)) onDropped(oldValue)
|
||||
}
|
||||
}
|
||||
|
||||
fun register(
|
||||
id: String,
|
||||
entry: T,
|
||||
) {
|
||||
idle.put(id, entry)
|
||||
}
|
||||
|
||||
fun setPlaying(
|
||||
id: String,
|
||||
isPlaying: Boolean,
|
||||
) {
|
||||
val entry = get(id) ?: return
|
||||
if (isPlaying) {
|
||||
playing[id] = entry
|
||||
} else {
|
||||
// Re-inserts if it was evicted from idle while playing. Must happen before the playing
|
||||
// entry is cleared, so the eviction this put may trigger still sees the pin.
|
||||
idle.put(id, entry)
|
||||
playing.remove(id)
|
||||
}
|
||||
}
|
||||
|
||||
fun get(id: String): T? = playing[id] ?: idle.get(id)
|
||||
|
||||
fun idleSnapshot(): List<T> = idle.snapshot().values.toList()
|
||||
|
||||
// Copies, like idleSnapshot(). Handing out playing.values would be a live view, and a caller
|
||||
// that iterated it while a drop fired would get a ConcurrentModificationException. A type whose
|
||||
// job is owning reachability should not ship that footgun.
|
||||
fun playingEntries(): List<T> = playing.values.toList()
|
||||
|
||||
/**
|
||||
* Explicit release. Drops whether or not the session is playing, and does not rely on
|
||||
* [idle]'s removal firing the callback — that reliance is the leak, because it silently does
|
||||
* nothing when the entry was already evicted while playing.
|
||||
*/
|
||||
fun drop(id: String): T? {
|
||||
val entry = playing.remove(id) ?: idle.get(id) ?: return null
|
||||
suppressDropSignal = true
|
||||
try {
|
||||
idle.remove(id)
|
||||
} finally {
|
||||
suppressDropSignal = false
|
||||
}
|
||||
onDropped(entry)
|
||||
return entry
|
||||
}
|
||||
|
||||
/** Teardown sweep: every reachable entry is dropped exactly once, playing or idle. */
|
||||
fun dropAll() {
|
||||
val all = ArrayList<T>()
|
||||
playing.values.forEach { entry -> if (all.none { it === entry }) all.add(entry) }
|
||||
idle.snapshot().values.forEach { entry -> if (all.none { it === entry }) all.add(entry) }
|
||||
|
||||
playing.clear()
|
||||
suppressDropSignal = true
|
||||
try {
|
||||
idle.evictAll()
|
||||
} finally {
|
||||
suppressDropSignal = false
|
||||
}
|
||||
|
||||
all.forEach(onDropped)
|
||||
}
|
||||
}
|
||||
+165
@@ -0,0 +1,165 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.service.playback.playerPool
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNotSame
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertSame
|
||||
import org.junit.Test
|
||||
|
||||
class SessionRegistryTest {
|
||||
private val dropped = mutableListOf<String>()
|
||||
|
||||
private fun registry(maxIdle: Int) = SessionRegistry<String>(maxIdle) { dropped.add(it) }
|
||||
|
||||
@Test
|
||||
fun evictionDropsTheOldestExactlyOnce() {
|
||||
val registry = registry(maxIdle = 1)
|
||||
registry.register("a", "A")
|
||||
registry.register("b", "B")
|
||||
assertEquals(listOf("A"), dropped)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun playingSessionSurvivesEviction() {
|
||||
val registry = registry(maxIdle = 1)
|
||||
registry.register("a", "A")
|
||||
registry.setPlaying("a", true)
|
||||
registry.register("b", "B")
|
||||
assertEquals(emptyList<String>(), dropped)
|
||||
}
|
||||
|
||||
// The replacement guard. setPlaying(false) re-puts the SAME entry, which fires
|
||||
// entryRemoved(newValue === oldValue). Treating that as a drop would retire the
|
||||
// session we are trying to keep.
|
||||
@Test
|
||||
fun pauseDoesNotDropTheSessionItIsKeeping() {
|
||||
val registry = registry(maxIdle = 2)
|
||||
registry.register("a", "A")
|
||||
registry.setPlaying("a", true)
|
||||
registry.setPlaying("a", false)
|
||||
assertEquals(emptyList<String>(), dropped)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun pausedSessionBecomesEvictableAgain() {
|
||||
val registry = registry(maxIdle = 1)
|
||||
registry.register("a", "A")
|
||||
registry.setPlaying("a", true)
|
||||
registry.register("b", "B") // A survives, but leaves idle
|
||||
registry.setPlaying("a", false) // A re-enters idle and displaces B
|
||||
assertEquals(listOf("B"), dropped)
|
||||
}
|
||||
|
||||
// Leak path 2: an explicit release must drop even while playing.
|
||||
@Test
|
||||
fun explicitDropOfPlayingSessionDropsExactlyOnce() {
|
||||
val registry = registry(maxIdle = 2)
|
||||
registry.register("a", "A")
|
||||
registry.setPlaying("a", true)
|
||||
registry.drop("a")
|
||||
assertEquals(listOf("A"), dropped)
|
||||
}
|
||||
|
||||
// Leak path 2, the exact case the old cache.remove(id) missed: already evicted
|
||||
// from idle while playing, so there is no cache entry left to trigger the callback.
|
||||
@Test
|
||||
fun explicitDropAfterEvictionWhilePlayingStillDrops() {
|
||||
val registry = registry(maxIdle = 1)
|
||||
registry.register("a", "A")
|
||||
registry.setPlaying("a", true)
|
||||
registry.register("b", "B")
|
||||
registry.drop("a")
|
||||
assertEquals(listOf("A"), dropped)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun dropAllDropsEveryEntryExactlyOnce() {
|
||||
val registry = registry(maxIdle = 3)
|
||||
registry.register("a", "A")
|
||||
registry.register("b", "B")
|
||||
registry.register("c", "C")
|
||||
registry.setPlaying("b", true)
|
||||
registry.dropAll()
|
||||
assertEquals(3, dropped.size)
|
||||
assertEquals(setOf("A", "B", "C"), dropped.toSet())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun dropAllIncludesSessionEvictedWhilePlaying() {
|
||||
val registry = registry(maxIdle = 1)
|
||||
registry.register("a", "A")
|
||||
registry.setPlaying("a", true)
|
||||
registry.register("b", "B")
|
||||
registry.dropAll()
|
||||
assertEquals(2, dropped.size)
|
||||
assertEquals(setOf("A", "B"), dropped.toSet())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun getFindsPlayingAndIdleEntries() {
|
||||
val registry = registry(maxIdle = 2)
|
||||
registry.register("a", "A")
|
||||
registry.register("b", "B")
|
||||
registry.setPlaying("a", true)
|
||||
assertSame("A", registry.get("a"))
|
||||
assertSame("B", registry.get("b"))
|
||||
assertNull(registry.get("missing"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun droppingUnknownIdIsANoOp() {
|
||||
val registry = registry(maxIdle = 2)
|
||||
assertNull(registry.drop("missing"))
|
||||
assertEquals(emptyList<String>(), dropped)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun enumerationSeesIdleAndPlayingSeparately() {
|
||||
val registry = registry(maxIdle = 2)
|
||||
registry.register("a", "A")
|
||||
registry.register("b", "B")
|
||||
registry.setPlaying("a", true)
|
||||
assertEquals(setOf("A", "B"), registry.idleSnapshot().toSet())
|
||||
assertEquals(listOf("A"), registry.playingEntries().toList())
|
||||
}
|
||||
|
||||
// The replacement guard uses !== (identity, not equality). This test pins that distinction:
|
||||
// replacing an entry with an equal-but-not-identical instance must drop the old one.
|
||||
// Changing !== to != would break this test, even though all existing tests would pass.
|
||||
@Test
|
||||
fun replacementWithEqualButDifferentInstanceDropsOldEntry() {
|
||||
val registry = registry(maxIdle = 2)
|
||||
val original = "A"
|
||||
registry.register("a", original)
|
||||
|
||||
// Create a new String instance that is equal to but not identical to the original.
|
||||
// String literals are interned, so we must construct it explicitly.
|
||||
val replacement = buildString { append("A") }
|
||||
assertEquals("Equal values", original, replacement)
|
||||
assertNotSame("Not the same instance", original, replacement)
|
||||
|
||||
// Registering the replacement should drop the original.
|
||||
registry.register("a", replacement)
|
||||
assertEquals(listOf(original), dropped)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user