From db2541feffe7f74fbf848d5a707f8e9c8eafc7e1 Mon Sep 17 00:00:00 2001 From: justinzeus Date: Fri, 6 Nov 2026 14:05:01 +0100 Subject: [PATCH] Move person-switch client disposal off the main thread --- .../wolfskin/tv/FakeConductarr.kt | 4 + .../wolfskin/tv/PersonSwitchThreadingTest.kt | 252 ++++++++++++++++++ .../wolfskin/tv/ShellStartupTimingTest.kt | 5 +- .../wolfskin/tv/TitleAsyncFocusTest.kt | 7 +- .../wolfskin/tv/ConductarrPreference.kt | 17 +- .../wolfskin/tv/ConductarrSession.kt | 81 ++++-- .../wolfskin/tv/HomeDestination.kt | 4 +- .../wolfskin/tv/TitleDestination.kt | 4 +- .../justinvisser/wolfskin/tv/WolfskinApp.kt | 64 +++-- .../wolfskin/tv/WolfskinLibrary.kt | 5 +- .../wolfskin/tv/read/HomeLibraryCache.kt | 2 +- docker/README.md | 16 ++ 12 files changed, 404 insertions(+), 57 deletions(-) create mode 100644 app/src/test/java/org/justinvisser/wolfskin/tv/PersonSwitchThreadingTest.kt diff --git a/app/src/test/java/org/justinvisser/wolfskin/tv/FakeConductarr.kt b/app/src/test/java/org/justinvisser/wolfskin/tv/FakeConductarr.kt index 04a8ac97..f741223e 100644 --- a/app/src/test/java/org/justinvisser/wolfskin/tv/FakeConductarr.kt +++ b/app/src/test/java/org/justinvisser/wolfskin/tv/FakeConductarr.kt @@ -11,6 +11,7 @@ import mockwebserver3.RecordedRequest internal const val SHARED_TOKEN = "0123456789abcdef0123456789abcdef" internal const val MINDEL_TOKEN = "fedcba9876543210fedcba9876543210" +internal const val JUSTIN_TOKEN = "11111111111111111111111111111111" /** A loopback server supplies wire JSON, never app projections or live service state. */ internal class FakeConductarr : Closeable { @@ -75,6 +76,9 @@ internal class FakeConductarr : Closeable { if (key == "operator" && request.headers["Authorization"]?.contains(MINDEL_TOKEN) == true) { body = JsonObject(body + mapOf("name" to JsonPrimitive("Mindel"), "requester" to JsonPrimitive("mindel"), "subject" to JsonPrimitive("jellyfin:mindel"), "watch_user" to JsonPrimitive("mindel"))) } + if (key == "operator" && request.headers["Authorization"]?.contains(JUSTIN_TOKEN) == true) { + body = JsonObject(body + mapOf("name" to JsonPrimitive("Justin"), "requester" to JsonPrimitive("justin"), "subject" to JsonPrimitive("jellyfin:justin"), "watch_user" to JsonPrimitive("justin"))) + } if (key in setOf("options", "discover") && request.url.queryParameter("media_type") == "tv") { body = JsonObject(body + ("media_type" to JsonPrimitive("tv"))) if (key == "discover") body = JsonObject(body + ("items" to JsonArray(listOf(JsonObject(body.getValue("items").jsonArray.first().jsonObject + mapOf("media_type" to JsonPrimitive("tv"), "tmdb_id" to JsonPrimitive(44), "title" to JsonPrimitive("Dark"))))))) diff --git a/app/src/test/java/org/justinvisser/wolfskin/tv/PersonSwitchThreadingTest.kt b/app/src/test/java/org/justinvisser/wolfskin/tv/PersonSwitchThreadingTest.kt new file mode 100644 index 00000000..20ea6d9b --- /dev/null +++ b/app/src/test/java/org/justinvisser/wolfskin/tv/PersonSwitchThreadingTest.kt @@ -0,0 +1,252 @@ +package org.justinvisser.wolfskin.tv + +import android.app.Application +import android.os.Looper +import androidx.lifecycle.viewModelScope +import androidx.test.core.app.ApplicationProvider +import com.github.damontecres.wholphin.data.CurrentUser +import com.github.damontecres.wholphin.data.ServerRepository +import com.github.damontecres.wholphin.data.model.* +import com.github.damontecres.wholphin.services.SetupDestination +import com.github.damontecres.wholphin.services.SetupNavigationManager +import io.mockk.* +import java.util.UUID +import java.util.concurrent.CopyOnWriteArrayList +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import kotlinx.coroutines.* +import kotlinx.coroutines.flow.last +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.setMain +import org.junit.After +import org.junit.Assert.* +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.justinvisser.wolfskin.conductarr.* +import org.justinvisser.wolfskin.design.ShellKey +import org.justinvisser.wolfskin.tv.read.LibraryReadFacade +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** Picker changes invalidate identity immediately without closing sockets on the UI thread. */ +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34], application = Application::class) +class PersonSwitchThreadingTest { + private val context = ApplicationProvider.getApplicationContext() + private lateinit var previousUrl: String + private var previousFixtures = false + + @Before fun setUp() { + Dispatchers.setMain(Dispatchers.Unconfined) + ConductarrPreference.initialize(context) + previousUrl = ConductarrPreference.baseUrl + previousFixtures = ConductarrPreference.fixtures + ConductarrPreference.updateFixtures(false) + } + + @After fun tearDown() { + ConductarrPreference.updateBaseUrl(previousUrl) + ConductarrPreference.updateFixtures(previousFixtures) + Dispatchers.resetMain() + unmockkConstructor(ConductarrClient::class) + } + + @Test fun pickerSwitchesKeepPersonDataAndDisposeEveryClientOffMainWithoutWaiting() = runBlocking { + assertSame(Looper.getMainLooper(), Looper.myLooper()) + val closeThreads = CopyOnWriteArrayList() + val releaseCloses = CountDownLatch(1) + val closed = CountDownLatch(4) + val disposalErrors = CopyOnWriteArrayList() + val disposalScope = CoroutineScope(SupervisorJob() + Dispatchers.IO + CoroutineExceptionHandler { _, error -> disposalErrors.add(error) }) + val failFirstClose = java.util.concurrent.atomic.AtomicBoolean(true) + mockkConstructor(ConductarrClient::class) + every { anyConstructed().close() } answers { + val onMain = Looper.myLooper() == Looper.getMainLooper() + closeThreads.add(onMain) + check(!onMain) { "StrictMode: Conductarr socket close on main" } + check(releaseCloses.await(10, TimeUnit.SECONDS)) { "Picker waited for socket disposal" } + callOriginal() + closed.countDown() + if (failFirstClose.getAndSet(false)) throw java.io.IOException("Socket disposal fixture failure") + } + FakeConductarr().use { fake -> + ConductarrPreference.updateBaseUrl(fake.url) + val conductarr = ConductarrSession(MemoryCacheStore(), disposalScope) + val server = JellyfinServer(UUID.randomUUID(), "Jellyfin", fake.url, null) + val justin = JellyfinUser(id = UUID.randomUUID(), serverId = server.id, name = "Justin", accessToken = JUSTIN_TOKEN) + val users = listOf(justin, justin.copy(id = UUID.randomUUID(), name = "Mindel", accessToken = MINDEL_TOKEN), + justin.copy(id = UUID.randomUUID(), name = "Shared", accessToken = SHARED_TOKEN)) + val repository = mockk(relaxed = true) + coEvery { repository.serverDao.getServer(server.id) } returns JellyfinServerUsers(server, users) + coEvery { repository.changeUser(server, any()) } coAnswers { CurrentUser(server, secondArg()) } + val setup = mockk(relaxed = true) + val libraries = mockk(relaxed = true) + coEvery { libraries.readDestinations(any()) } returns emptyMap() + val session = WolfskinSession() + val model = WolfskinShellViewModel(context, session, repository, mockk(relaxed = true), setup, mockk(relaxed = true), + libraries, WolfskinSeeAllLists(), conductarr, mockk(relaxed = true)) + val clients = mutableListOf() + try { + model.load(server, justin) + requireNotNull(model.onForeground(server, justin)).join() + clients.add(conductarr.identity.value!!.client) + val shell = session.state!! + shell.key(ShellKey.Ok) + for (index in listOf(1, 2, 0)) { + val switched = CompletableDeferred() + every { setup.navigateTo(any()) } answers { switched.complete((firstArg() as SetupDestination.AppContent).current) } + shell.focusNav(shell.tabs.size) + shell.key(ShellKey.Ok) + shell.focusPerson(index) + shell.key(ShellKey.Ok) + assertEquals(users[index].id.toString(), shell.person.jellyfinUserId) + assertEquals("home", shell.route) + assertFalse(shell.pickerVisible) + assertNull("Old identity must not serve the newly selected person", conductarr.identity.value) + val current = withTimeout(5_000) { switched.await() } + assertEquals(users[index].id, current.user.id) + model.load(current.server, current.user) + requireNotNull(model.onForeground(server, current.user)).join() + assertEquals(users[index].name!!.lowercase(), conductarr.identity.value!!.operator!!.requester) + assertEquals(users[index].name!!.lowercase(), model.signedInPerson) + clients.add(conductarr.identity.value!!.client) + } + conductarr.close() + conductarr.close() + model.viewModelScope.cancel() + releaseCloses.countDown() + assertTrue("Every retired client must finish disposal", withContext(Dispatchers.IO) { closed.await(5, TimeUnit.SECONDS) }) + assertEquals(listOf(false, false, false, false), closeThreads.toList()) + for (client in clients) { + val result = client.read(TvRoutes.operator(), ReadTrigger.FORCE).last() + assertEquals("Retired clients cannot start another network read", ClientError.Unreachable, (result as ReadResult.Failure).error) + } + disposalScope.coroutineContext.job.children.toList().joinAll() + assertTrue("A failing socket close must be contained", disposalErrors.isEmpty()) + } finally { + releaseCloses.countDown() + model.viewModelScope.cancel() + withContext(Dispatchers.IO) { conductarr.close() } + disposalScope.coroutineContext.job.children.toList().joinAll() + disposalScope.cancel() + } + } + } + + @Test fun rapidPickerChoicesIgnoreLateSignInAndCanReturnToTheOriginalPerson() = runBlocking { + FakeConductarr().use { fake -> + ConductarrPreference.updateBaseUrl(fake.url) + ConductarrSession(MemoryCacheStore()).use { conductarr -> + val server = JellyfinServer(UUID.randomUUID(), "Jellyfin", fake.url, null) + val justin = JellyfinUser(id = UUID.randomUUID(), serverId = server.id, name = "Justin", accessToken = JUSTIN_TOKEN) + val users = listOf(justin, justin.copy(id = UUID.randomUUID(), name = "Mindel", accessToken = MINDEL_TOKEN), + justin.copy(id = UUID.randomUUID(), name = "Shared", accessToken = SHARED_TOKEN)) + val repository = mockk(relaxed = true) + coEvery { repository.serverDao.getServer(server.id) } returns JellyfinServerUsers(server, users) + val pending = CompletableDeferred() + val release = CompletableDeferred() + coEvery { repository.changeUser(server, any()) } coAnswers { + val user = secondArg() + if (user.id == users[1].id) { + pending.complete(Unit) + withContext(NonCancellable) { release.await() } + } + CurrentUser(server, user) + } + val switched = CompletableDeferred() + val setup = mockk(relaxed = true) + every { setup.navigateTo(any()) } answers { switched.complete((firstArg() as SetupDestination.AppContent).current) } + val libraries = mockk(relaxed = true) + coEvery { libraries.readDestinations(any()) } returns emptyMap() + val session = WolfskinSession() + val model = WolfskinShellViewModel(context, session, repository, mockk(relaxed = true), setup, mockk(relaxed = true), + libraries, WolfskinSeeAllLists(), conductarr, mockk(relaxed = true)) + try { + model.load(server, justin) + requireNotNull(model.onForeground(server, justin)).join() + val shell = session.state!! + shell.key(ShellKey.Ok) + for (index in listOf(1, 2, 0)) { + shell.focusNav(shell.tabs.size) + shell.key(ShellKey.Ok) + shell.focusPerson(index) + shell.key(ShellKey.Ok) + if (index == 1) withTimeout(5_000) { pending.await() } + } + release.complete(Unit) + val current = withTimeout(5_000) { switched.await() } + assertEquals(justin.id, current.user.id) + verify(exactly = 1) { setup.navigateTo(any()) } + model.load(server, current.user) + requireNotNull(model.onForeground(server, current.user)).join() + assertEquals(justin.id.toString(), shell.person.jellyfinUserId) + assertEquals("justin", model.signedInPerson) + assertEquals("justin", conductarr.identity.value!!.operator!!.requester) + } finally { + release.complete(Unit) + model.viewModelScope.cancel() + } + } + } + } + + @Test fun homeAndTitleRetriesReplaceSignedOutClientsWithoutMainThreadDisposal() = runBlocking { + val closeThreads = CopyOnWriteArrayList() + val closed = CountDownLatch(3) + val disposalScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) + mockkConstructor(ConductarrClient::class) + every { anyConstructed().close() } answers { + val onMain = Looper.myLooper() == Looper.getMainLooper() + closeThreads.add(onMain) + check(!onMain) { "StrictMode: retry socket close on main" } + callOriginal() + closed.countDown() + } + FakeConductarr().use { fake -> + ConductarrPreference.updateBaseUrl(fake.url) + val conductarr = ConductarrSession(MemoryCacheStore(), disposalScope) + val server = JellyfinServer(UUID.randomUUID(), "Jellyfin", fake.url, null) + val user = JellyfinUser(id = UUID.randomUUID(), serverId = server.id, name = "Justin", accessToken = JUSTIN_TOKEN) + val repository = mockk() + every { repository.currentUser } returns user + every { repository.currentServer } returns server + val home = mockk() + coEvery { home.read(user.id) } returns org.justinvisser.wolfskin.design.HomeLibrary(resume = listOf( + org.justinvisser.wolfskin.design.HomeTitle("justin-film", "Justin film", "movie", held = true))) + val cache = mockk(relaxed = true) + every { cache.read(any()) } returns null + val homeModel = WolfskinHomeViewModel(context, home, cache, mockk(relaxed = true), WolfskinSeeAllLists(), conductarr) + val titleModel = WolfskinTitleViewModel(context, mockk(), mockk(), mockk(relaxed = true), conductarr, repository) + try { + conductarr.select("${server.id}:${user.id}", user.accessToken, fake.url) + conductarr.authenticate() + fake.status = 401 + conductarr.authenticate() + assertFalse(conductarr.identity.value!!.signedIn) + fake.status = 200 + homeModel.load(server, user, org.justinvisser.wolfskin.design.ShellPerson("justin", "Justin", "J", false, user.id.toString()), retry = true) + withTimeout(5_000) { conductarr.identity.first { it?.signedIn == true } } + assertEquals("justin", conductarr.identity.value!!.operator!!.requester) + assertTrue(homeModel.snapshot.rows.any { row -> row.cards.any { it.id == "justin-film" } }) + fake.status = 401 + conductarr.authenticate() + assertFalse(conductarr.identity.value!!.signedIn) + fake.status = 200 + titleModel.retrySignIn() + assertEquals("justin", conductarr.identity.value!!.operator!!.requester) + assertTrue(conductarr.identity.value!!.signedIn) + } finally { + homeModel.viewModelScope.cancel() + titleModel.viewModelScope.cancel() + conductarr.close() + disposalScope.coroutineContext.job.children.toList().joinAll() + disposalScope.cancel() + } + assertEquals(0L, closed.count) + assertEquals(listOf(false, false, false), closeThreads.toList()) + } + } +} diff --git a/app/src/test/java/org/justinvisser/wolfskin/tv/ShellStartupTimingTest.kt b/app/src/test/java/org/justinvisser/wolfskin/tv/ShellStartupTimingTest.kt index c3bf121a..dbcffdb4 100644 --- a/app/src/test/java/org/justinvisser/wolfskin/tv/ShellStartupTimingTest.kt +++ b/app/src/test/java/org/justinvisser/wolfskin/tv/ShellStartupTimingTest.kt @@ -58,8 +58,9 @@ class ShellStartupTimingTest { val activity = async(Dispatchers.IO) { conductarr.identity.value!!.provider.requests().last() } withTimeout(5_000) { activityEntered.await() } val savedPeople = CompletableDeferred() + val savedPeopleStarted = CompletableDeferred() val repository = mockk(relaxed = true) - coEvery { repository.serverDao.getServer(server.id) } coAnswers { savedPeople.await(); null } + coEvery { repository.serverDao.getServer(server.id) } coAnswers { savedPeopleStarted.complete(Unit); savedPeople.await(); null } val navigation = mockk(relaxed = true) val libraries = mockk(relaxed = true) coEvery { libraries.readDestinations(user.id) } returns emptyMap() @@ -68,7 +69,7 @@ class ShellStartupTimingTest { libraries, WolfskinSeeAllLists(), conductarr, mockk(relaxed = true)) val load = launch { model.load(server, user) } try { - yield() + withTimeout(5_000) { savedPeopleStarted.await() } val shell = requireNotNull(session.state) { "The current Jellyfin person must paint before saved-login I/O" } assertEquals(PeopleStatus.Ready, shell.peopleStatus) assertEquals(user.id.toString(), shell.person.jellyfinUserId) diff --git a/app/src/test/java/org/justinvisser/wolfskin/tv/TitleAsyncFocusTest.kt b/app/src/test/java/org/justinvisser/wolfskin/tv/TitleAsyncFocusTest.kt index 12ffa9e8..9809967f 100644 --- a/app/src/test/java/org/justinvisser/wolfskin/tv/TitleAsyncFocusTest.kt +++ b/app/src/test/java/org/justinvisser/wolfskin/tv/TitleAsyncFocusTest.kt @@ -1,6 +1,7 @@ package org.justinvisser.wolfskin.tv import android.app.Application +import android.os.Looper import androidx.activity.ComponentActivity import androidx.compose.ui.input.key.Key import androidx.compose.ui.test.junit4.createAndroidComposeRule @@ -35,6 +36,7 @@ import org.justinvisser.wolfskin.design.TitleVersion import org.justinvisser.wolfskin.design.WolfskinTheme import org.justinvisser.wolfskin.tv.read.TitleReadFacade import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows import org.robolectric.annotation.Config import java.util.UUID @@ -152,7 +154,10 @@ class TitleAsyncFocusTest { assertEquals("retry", viewModel.title!!.options[viewModel.title!!.column].id) rule.runOnIdle { viewModel.title!!.toast = null } press(Key.DirectionCenter) - rule.waitUntil(timeoutMillis = 5000) { reads.get() == 2 && viewModel.title?.toast != null } + rule.waitUntil(timeoutMillis = 5000) { + Shadows.shadowOf(Looper.getMainLooper()).idle() + reads.get() == 2 && viewModel.title?.toast != null + } rule.waitForIdle() assertEquals("Saved title", viewModel.title!!.data.item.name) assertEquals("20:42", shell.navSavedAt) diff --git a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrPreference.kt b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrPreference.kt index f15fdef7..559b4365 100644 --- a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrPreference.kt +++ b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrPreference.kt @@ -9,6 +9,7 @@ import androidx.compose.runtime.setValue import androidx.core.content.edit import androidx.preference.PreferenceManager import com.github.damontecres.wholphin.BuildConfig +import com.github.damontecres.wholphin.services.hilt.IoCoroutineScope import dagger.Module import dagger.Provides import dagger.hilt.InstallIn @@ -16,6 +17,9 @@ import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.components.SingletonComponent import java.io.File import javax.inject.Singleton +import kotlinx.coroutines.CoroutineScope +import org.justinvisser.wolfskin.conductarr.CacheEntry +import org.justinvisser.wolfskin.conductarr.CacheStore import org.justinvisser.wolfskin.conductarr.FileCacheStore /** Advanced-only endpoint and fixture controls contain no credentials. */ @@ -50,6 +54,15 @@ object ConductarrPreference { object ConductarrModule { @Provides @Singleton - fun session(@ApplicationContext context: Context): ConductarrSession = - ConductarrSession(FileCacheStore(File(context.noBackupFilesDir, "conductarr"))) + fun session(@ApplicationContext context: Context, @IoCoroutineScope scope: CoroutineScope): ConductarrSession = + ConductarrSession(ConductarrDiskCache(context), scope) +} + +/** Android may create noBackupFilesDir, so resolve it only during an IO cache operation. */ +private class ConductarrDiskCache(context: Context) : CacheStore { + private val store by lazy { FileCacheStore(File(context.noBackupFilesDir, "conductarr")) } + override fun read(scope: String, key: String) = store.read(scope, key) + override fun write(scope: String, key: String, entry: CacheEntry, capacity: Int?) = store.write(scope, key, entry, capacity) + override fun remove(scope: String, key: String) = store.remove(scope, key) + override fun clear(scope: String) = store.clear(scope) } diff --git a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrSession.kt b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrSession.kt index 64cdba69..5ba52caa 100644 --- a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrSession.kt +++ b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrSession.kt @@ -3,12 +3,17 @@ package org.justinvisser.wolfskin.tv import java.io.Closeable import java.security.MessageDigest +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.launch import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.update import org.justinvisser.wolfskin.conductarr.* import org.justinvisser.wolfskin.conductarr.model.Operator +import timber.log.Timber /** A selected Jellyfin login owns every provider and all in-memory observations. */ data class ConductarrIdentity( @@ -21,53 +26,85 @@ data class ConductarrIdentity( ) /** Tokens are borrowed from upstream, never persisted or exposed as observable state. */ -class ConductarrSession(private val cache: CacheStore) : Closeable { +class ConductarrSession( + private val cache: CacheStore, + private val disposalScope: CoroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.IO), +) : Closeable { private val mutableIdentity = MutableStateFlow(null) val identity: StateFlow = mutableIdentity.asStateFlow() private var selection: ByteArray? = null @Volatile private var generation = 0L - @Synchronized fun select(person: String, token: String?, baseUrl: String, retry: Boolean = false) { val fingerprint = MessageDigest.getInstance("SHA-256") .digest("$person\n$baseUrl\n${token.orEmpty()}".toByteArray()) - if (!retry && selection?.contentEquals(fingerprint) == true) return - generation++ - val currentGeneration = generation - mutableIdentity.value?.client?.close() - mutableIdentity.value = null - selection = fingerprint - if (token.isNullOrBlank()) return - val client = try { - ConductarrClient(baseUrl, token, PersonCacheStore(cache, person), timeouts = ClientTimeouts(3, 5, 8), onEvent = { event -> - if (event == ClientEvent.NEEDS_SIGN_IN && generation == currentGeneration) { - mutableIdentity.update { it?.copy(operator = null, signedIn = false, error = ClientError.NeedsSignIn) } - } - }) - } catch (_: IllegalArgumentException) { return } - mutableIdentity.value = ConductarrIdentity(person, client, ConductarrProvider(client)) + val previous: ConductarrClient? + val currentGeneration: Long + synchronized(this) { + if (!retry && selection?.contentEquals(fingerprint) == true) return + generation++ + currentGeneration = generation + previous = mutableIdentity.value?.client + mutableIdentity.value = null + selection = fingerprint + } + try { + val client = if (token.isNullOrBlank()) null else try { + ConductarrClient(baseUrl, token, PersonCacheStore(cache, person), timeouts = ClientTimeouts(3, 5, 8), onEvent = { event -> + if (event == ClientEvent.NEEDS_SIGN_IN) { + mutableIdentity.update { + if (generation == currentGeneration) it?.copy(operator = null, signedIn = false, error = ClientError.NeedsSignIn) else it + } + } + }) + } catch (_: IllegalArgumentException) { null } + synchronized(this) { + if (generation == currentGeneration) { + mutableIdentity.value = client?.let { ConductarrIdentity(person, it, ConductarrProvider(it)) } + } else dispose(client) + } + } finally { + dispose(previous) + } } /** Disk identity paints first; a 401 always removes it, even after an earlier saved emission. */ suspend fun authenticate() { val selected = mutableIdentity.value ?: return selected.client.read(TvRoutes.operator(), ReadTrigger.LAUNCH).collect { result -> - if (mutableIdentity.value?.client !== selected.client) return@collect when (result) { - is ReadResult.Value -> mutableIdentity.update { it?.copy(operator = result.saved.value, signedIn = result.saved.value != null, error = null) } + is ReadResult.Value -> mutableIdentity.update { + if (it?.client === selected.client) it.copy(operator = result.saved.value, signedIn = result.saved.value != null, error = null) else it + } is ReadResult.Failure -> mutableIdentity.update { - if (result.error == ClientError.NeedsSignIn) it?.copy(operator = null, signedIn = false, error = result.error) - else it?.copy(error = result.error) + if (it?.client !== selected.client) it + else if (result.error == ClientError.NeedsSignIn) it.copy(operator = null, signedIn = false, error = result.error) + else it.copy(error = result.error) } } } } + /** Remove read authority synchronously; socket disposal finishes independently on IO. */ + @Synchronized override fun close() { generation++ - mutableIdentity.value?.client?.close() + val previous = mutableIdentity.value?.client mutableIdentity.value = null selection = null + dispose(previous) + } + + private fun dispose(client: ConductarrClient?) { + if (client == null) return + // The application scope outlives the screen replaced by a person switch. + disposalScope.launch(Dispatchers.IO) { + try { + client.close() + } catch (_: Exception) { + Timber.w("Conductarr client disposal failed") + } + } } } diff --git a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/HomeDestination.kt b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/HomeDestination.kt index 7687e595..c8ab24cc 100644 --- a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/HomeDestination.kt +++ b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/HomeDestination.kt @@ -52,7 +52,9 @@ class WolfskinHomeViewModel @Inject constructor( catalogueJob?.cancel() withContext(Dispatchers.IO) { debugHomeSnapshot(context, person.key) }?.let { snapshot = it; return } if (retry && conductarr.identity.value?.signedIn != true) { - conductarr.select("${server.id}:${user.id}", user.accessToken, ConductarrPreference.baseUrl, retry = true) + withContext(Dispatchers.IO) { + conductarr.select("${server.id}:${user.id}", user.accessToken, ConductarrPreference.baseUrl, retry = true) + } } loadLibrary(server, user, person, retry) } diff --git a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/TitleDestination.kt b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/TitleDestination.kt index 691e660f..e457aa86 100644 --- a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/TitleDestination.kt +++ b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/TitleDestination.kt @@ -210,7 +210,9 @@ class WolfskinTitleViewModel @Inject constructor( internal suspend fun retrySignIn() { val user = repository.currentUser ?: return val server = repository.currentServer ?: return - conductarr.select("${server.id}:${user.id}", user.accessToken, ConductarrPreference.baseUrl, retry = true) + withContext(Dispatchers.IO) { + conductarr.select("${server.id}:${user.id}", user.accessToken, ConductarrPreference.baseUrl, retry = true) + } conductarr.authenticate() } diff --git a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/WolfskinApp.kt b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/WolfskinApp.kt index a4859d21..abc0089c 100644 --- a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/WolfskinApp.kt +++ b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/WolfskinApp.kt @@ -39,6 +39,9 @@ import dagger.hilt.android.qualifiers.ApplicationContext import javax.inject.Inject import javax.inject.Singleton import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.launch import org.jellyfin.sdk.model.serializer.toUUIDOrNull @@ -85,23 +88,29 @@ class WolfskinShellViewModel @Inject constructor( var signedInPerson by mutableStateOf("") private set private var identityJob: kotlinx.coroutines.Job? = null + private var personSwitchJob: kotlinx.coroutines.Job? = null + private val personSwitchMutex = Mutex() private var fixtureMode: Boolean? = null suspend fun load(server: JellyfinServer, user: JellyfinUser) { - WolfskinScreenPreference.initialize(context) - ConductarrPreference.initialize(context) + withContext(Dispatchers.IO) { + WolfskinScreenPreference.initialize(context) + ConductarrPreference.initialize(context) + } identityJob?.cancel() libraries.audiences = null if (ConductarrPreference.fixtures) conductarr.close() - else conductarr.select("${server.id}:${user.id}", user.accessToken, ConductarrPreference.baseUrl) + else withContext(Dispatchers.IO) { conductarr.select("${server.id}:${user.id}", user.accessToken, ConductarrPreference.baseUrl) } signedInPerson = session.state?.people?.firstOrNull { it.jellyfinUserId == user.id.toString() }?.key ?: user.name.orEmpty().lowercase() session.onAction = { action -> handleAction(action, server, user) } if (session.state == null || session.serverId != server.id.toString() || fixtureMode != ConductarrPreference.fixtures) { session.serverId = server.id.toString() fixtureMode = ConductarrPreference.fixtures - session.state = ShellState(readShellPeople(context, listOf(user)), signedInPerson) { session.onAction(it) } + val people = withContext(Dispatchers.IO) { readShellPeople(context, listOf(user)) } + session.state = ShellState(people, signedInPerson) { session.onAction(it) } val saved = withContext(Dispatchers.IO) { repository.serverDao.getServer(server.id)?.users.orEmpty() } - session.state?.updatePeople(readShellPeople(context, saved.ifEmpty { listOf(user) })) + val savedPeople = withContext(Dispatchers.IO) { readShellPeople(context, saved.ifEmpty { listOf(user) }) } + session.state?.updatePeople(savedPeople) } identityJob = viewModelScope.launch { if (!ConductarrPreference.fixtures) loadHousehold(user) @@ -149,8 +158,10 @@ class WolfskinShellViewModel @Inject constructor( } suspend fun retryConductarr(server: JellyfinServer, user: JellyfinUser) { - conductarr.select("${server.id}:${user.id}", user.accessToken, ConductarrPreference.baseUrl, - retry = conductarr.identity.value?.signedIn != true) + withContext(Dispatchers.IO) { + conductarr.select("${server.id}:${user.id}", user.accessToken, ConductarrPreference.baseUrl, + retry = conductarr.identity.value?.signedIn != true) + } loadHousehold(user) } @@ -191,7 +202,7 @@ class WolfskinShellViewModel @Inject constructor( } ShellAction.Pop -> navigation.goBack() is ShellAction.HandOff -> navigation.navigateTo(handOffDestination(action.group)) - is ShellAction.Person -> selectPerson(action.person, server, user) + is ShellAction.Person -> selectPerson(action.person, server) ShellAction.RetryPeople -> viewModelScope.launch { retryConductarr(server, user) } ShellAction.RetryRequests -> viewModelScope.launch { if (conductarr.identity.value?.signedIn != true) retryConductarr(server, user) @@ -201,26 +212,29 @@ class WolfskinShellViewModel @Inject constructor( } } - private fun selectPerson(person: ShellPerson, server: JellyfinServer, user: JellyfinUser) { + private fun selectPerson(person: ShellPerson, server: JellyfinServer) { navigation.replace(Destination.Home()) - if (person.jellyfinUserId == user.id.toString() || person.key == signedInPerson) return + if (person.key == signedInPerson) return + identityJob?.cancel() + personSwitchJob?.cancel() + signedInPerson = "" conductarr.close() libraries.audiences = null - viewModelScope.launch { - val saved = withContext(Dispatchers.IO) { - repository.serverDao.getServer(server.id)?.users?.firstOrNull { it.id.toString() == person.jellyfinUserId || it.name.equals(person.name, ignoreCase = true) } - } - if (saved == null || saved.isProtected) { - signedInPerson = "" - return@launch - } - try { - val current = repository.changeUser(server, saved) - setupNavigation.navigateTo(SetupDestination.AppContent(current)) - } catch (exception: Exception) { - if (exception is kotlinx.coroutines.CancellationException) throw exception - signedInPerson = "" - session.state?.apply { peopleStatus = PeopleStatus.Unavailable; pickerVisible = true } + personSwitchJob = viewModelScope.launch { + personSwitchMutex.withLock { + val saved = withContext(Dispatchers.IO) { + repository.serverDao.getServer(server.id)?.users?.firstOrNull { it.id.toString() == person.jellyfinUserId || it.name.equals(person.name, ignoreCase = true) } + } + if (saved == null || saved.isProtected) return@launch + try { + val current = repository.changeUser(server, saved) + coroutineContext.ensureActive() + setupNavigation.navigateTo(SetupDestination.AppContent(current)) + } catch (exception: Exception) { + if (exception is kotlinx.coroutines.CancellationException) throw exception + coroutineContext.ensureActive() + session.state?.apply { peopleStatus = PeopleStatus.Unavailable; pickerVisible = true } + } } } } diff --git a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/WolfskinLibrary.kt b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/WolfskinLibrary.kt index fec2c537..0301d3dc 100644 --- a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/WolfskinLibrary.kt +++ b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/WolfskinLibrary.kt @@ -59,7 +59,7 @@ fun WolfskinLibraryContent( context.assets.open("wolfskin-library.json").bufferedReader().use { LibraryFixture(it.readText(), "file:///android_asset/library/") } } } - val snapshots = remember(context) { LibrarySnapshotStore(FileCacheStore(File(context.filesDir, "wolfskin-library-cache"))) } + val snapshots = remember(context) { lazy { LibrarySnapshotStore(FileCacheStore(File(context.filesDir, "wolfskin-library-cache"))) } } val grid = remember(shell.person.key, route, view) { view.library ?: if (route.startsWith("seeall/")) seeAll.get(route.substringAfter("seeall/").takeIf { it.startsWith("${shell.person.key}/") } ?: "") else LibraryGridState( title = route.replaceFirstChar { it.uppercase() }, items = emptyList(), @@ -81,7 +81,8 @@ fun WolfskinLibraryContent( grid.source = withContext(Dispatchers.IO) { fixture!!.library(shell.person.key, route) } grid.status = LibraryStatus.Ready } else { - snapshots.read(cacheScope, route, force = grid.refreshRevision > 0) { facade.readLibrary(user.id, route) }.collect { snapshot -> + val store = withContext(Dispatchers.IO) { snapshots.value } + store.read(cacheScope, route, force = grid.refreshRevision > 0) { facade.readLibrary(user.id, route) }.collect { snapshot -> val focusedId = grid.focusedTitle?.id grid.source = snapshot.items focusedId?.let { id -> grid.items.indexOfFirst { it.id == id }.takeIf { it >= 0 }?.let { grid.index = it } } diff --git a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/read/HomeLibraryCache.kt b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/read/HomeLibraryCache.kt index 6e69ac54..10422c09 100644 --- a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/read/HomeLibraryCache.kt +++ b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/read/HomeLibraryCache.kt @@ -16,7 +16,7 @@ import org.justinvisser.wolfskin.design.HomeTitle /** Saved Jellyfin projections reuse the app-private atomic cache and are partitioned by server/user. */ @Singleton class HomeLibraryCache @Inject constructor(@param:ApplicationContext context: Context) { - private val store = FileCacheStore(File(context.filesDir, "wolfskin-home")) + private val store by lazy { FileCacheStore(File(context.filesDir, "wolfskin-home")) } fun read(scope: String): Pair? { val entry = store.read(scope, KEY) ?: return null diff --git a/docker/README.md b/docker/README.md index 583d4d4b..fe872463 100644 --- a/docker/README.md +++ b/docker/README.md @@ -104,6 +104,22 @@ household labels, requester capabilities and library audiences after sign-in. The locked shell projection in `app/src/debug/assets/wolfskin-shell.json` is used only when the Advanced debug fixture switch is enabled; release excludes it. +Person selection immediately clears the old Conductarr identity and Jellyfin +read gate before saved-login I/O. Detached clients are disposed exactly once on +the application's IO scope, after the replacement (or signed-out) identity is +published; a slow or failing socket close cannot block the picker or abort a +switch. Cleanup outlives the replaced screen. Retry and session replacement use +the same disposal owner. Superseded sign-ins are cancelled and serialized so a +late reply cannot navigate back to an earlier person. + +Preference and people-fixture reads, Conductarr client construction (including +Home/title retries), and private-cache directory initialization run on IO. +`PersonSwitchThreadingTest` replays the real picker across Justin → Mindel → +Shared → Justin while all closes are held pending, verifies retired clients +cannot start reads, contains a failing close, and checks a burst of choices +against a late sign-in reply. The thread-recording close fake rejects the main +Looper; actual TLS/StrictMode verification remains a real-device check. + Discover adds the picks hero, type chips, catalogue shelves, service pages, continuous See all grids, Hidden picks, request chooser and receipts. Its `discover-title/...` destination is an explicit title-family integration stub exposing only Discover's