danvics/Librechat-Mobile
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(data): mutex guard repository caches against suspension races

Browse source 
AgentRepositoryImpl.cachedAgents and PresetRepositoryImpl.cachedPresets
are read-checked, fetched across a suspension point, then written. With
@Volatile alone, two coroutines can both observe null, both issue the
network request, and both write the cache — a redundant API call per
cold-cache race. Wrap read-check-write in a Mutex so at most one fetch
runs per cold cache; invalidateCache() becomes suspend to match.
This commit is contained in:
Garfie 2026-04-27 18:32:07 -06:00
parent f236d97c19
commit ac125bb24c
3 changed files with 155 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

130
core/data/src/androidUnitTest/kotlin/com/garfiec/librechat/core/data/repository/RepositoryCacheMutexTest.kt Normal file
View file

@ -0,0 +1,130 @@
package com.garfiec.librechat.core.data.repository
import com.garfiec.librechat.core.common.result.Result
import com.garfiec.librechat.core.model.Agent
import com.garfiec.librechat.core.model.Preset
import com.garfiec.librechat.core.model.response.AgentListResponse
import com.garfiec.librechat.core.network.api.AgentsApi
import com.garfiec.librechat.core.network.api.PresetsApi
import com.google.common.truth.Truth.assertThat
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.mockk
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.async
import kotlinx.coroutines.delay
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.runTest
import org.junit.Test
@OptIn(ExperimentalCoroutinesApi::class)
class RepositoryCacheMutexTest {
@Test
fun `AgentRepositoryImpl - two concurrent getAgents on cold cache hit the API exactly once`() =
runTest {
val agentsApi = mockk<AgentsApi>()
val responseAgents = listOf(Agent(id = "a1"), Agent(id = "a2"))
coEvery { agentsApi.getAgents(null) } coAnswers {
delay(50)
AgentListResponse(data = responseAgents)
}
val repository = AgentRepositoryImpl(agentsApi = agentsApi)
val first = async { repository.getAgents(category = null) }
val second = async { repository.getAgents(category = null) }
advanceUntilIdle()
val firstResult = first.await()
val secondResult = second.await()
assertThat(firstResult).isInstanceOf(Result.Success::class.java)
assertThat(secondResult).isInstanceOf(Result.Success::class.java)
assertThat((firstResult as Result.Success).data).isEqualTo(responseAgents)
assertThat((secondResult as Result.Success).data).isEqualTo(responseAgents)
coVerify(exactly = 1) { agentsApi.getAgents(null) }
}
@Test
fun `AgentRepositoryImpl - second call after cache hydrated does not re-fetch`() = runTest {
val agentsApi = mockk<AgentsApi>()
val responseAgents = listOf(Agent(id = "a1"))
coEvery { agentsApi.getAgents(null) } returns AgentListResponse(data = responseAgents)
val repository = AgentRepositoryImpl(agentsApi = agentsApi)
repository.getAgents(category = null)
repository.getAgents(category = null)
repository.getAgents(category = null)
coVerify(exactly = 1) { agentsApi.getAgents(null) }
}
@Test
fun `AgentRepositoryImpl - categorised getAgents bypasses cache and does not populate it`() =
runTest {
val agentsApi = mockk<AgentsApi>()
val filteredAgents = listOf(Agent(id = "cat1"))
val allAgents = listOf(Agent(id = "a1"), Agent(id = "a2"))
coEvery { agentsApi.getAgents("writing") } returns AgentListResponse(data = filteredAgents)
coEvery { agentsApi.getAgents(null) } returns AgentListResponse(data = allAgents)
val repository = AgentRepositoryImpl(agentsApi = agentsApi)
repository.getAgents(category = "writing")
// Cache must still be cold — next call with category=null must hit the API.
repository.getAgents(category = null)
repository.getAgents(category = null)
coVerify(exactly = 1) { agentsApi.getAgents("writing") }
coVerify(exactly = 1) { agentsApi.getAgents(null) }
}
@Test
fun `PresetRepositoryImpl - two concurrent getAll on cold cache hit the API exactly once`() =
runTest {
val presetsApi = mockk<PresetsApi>()
val responsePresets = listOf(
Preset(presetId = "p1", title = "One"),
Preset(presetId = "p2", title = "Two"),
)
coEvery { presetsApi.getPresets() } coAnswers {
delay(50)
responsePresets
}
val repository = PresetRepositoryImpl(presetsApi = presetsApi)
val first = async { repository.getAll() }
val second = async { repository.getAll() }
advanceUntilIdle()
val firstResult = first.await()
val secondResult = second.await()
assertThat(firstResult).isInstanceOf(Result.Success::class.java)
assertThat(secondResult).isInstanceOf(Result.Success::class.java)
assertThat((firstResult as Result.Success).data).isEqualTo(responsePresets)
assertThat((secondResult as Result.Success).data).isEqualTo(responsePresets)
coVerify(exactly = 1) { presetsApi.getPresets() }
}
@Test
fun `PresetRepositoryImpl - mutation invalidates cache so next getAll re-fetches`() = runTest {
val presetsApi = mockk<PresetsApi>()
val first = listOf(Preset(presetId = "p1", title = "One"))
val second = listOf(
Preset(presetId = "p1", title = "One"),
Preset(presetId = "p2", title = "Two"),
)
coEvery { presetsApi.getPresets() } returnsMany listOf(first, second)
coEvery { presetsApi.createPreset(any()) } returns Preset(presetId = "p2", title = "Two")
val repository = PresetRepositoryImpl(presetsApi = presetsApi)
repository.getAll()
repository.create(Preset(presetId = "p2", title = "Two"))
val result = repository.getAll()
assertThat((result as Result.Success).data).isEqualTo(second)
coVerify(exactly = 2) { presetsApi.getPresets() }
}
}

23
core/data/src/commonMain/kotlin/com/garfiec/librechat/core/data/repository/AgentRepositoryImpl.kt
View file

@ -17,28 +17,30 @@ import com.garfiec.librechat.core.model.request.RevertAgentRequest
import com.garfiec.librechat.core.model.request.ToolCallRequest
import com.garfiec.librechat.core.model.request.UpdateAgentRequest
import com.garfiec.librechat.core.network.api.AgentsApi
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import kotlinx.serialization.json.Json
import kotlin.concurrent.Volatile
class AgentRepositoryImpl(
private val agentsApi: AgentsApi,
) : AgentRepository {
@Volatile
private val cacheMutex = Mutex()
private var cachedAgents: List<Agent>? = null
// --- Agent CRUD ---
override suspend fun getAgents(category: String?): Result<List<Agent>> {
return safeApiCall {
if (category == null && cachedAgents != null) {
return@safeApiCall cachedAgents!!
if (category != null) {
return@safeApiCall agentsApi.getAgents(category).data
}
val agents = agentsApi.getAgents(category).data
if (category == null) {
cacheMutex.withLock {
cachedAgents?.let { return@withLock it }
val agents = agentsApi.getAgents(null).data
cachedAgents = agents
agents
}
agents
}
}
@ -65,7 +67,8 @@ class AgentRepositoryImpl(
override suspend fun getAgent(id: String): Result<Agent> {
return safeApiCall {
cachedAgents?.find { it.id == id }?.let { return@safeApiCall it }
cacheMutex.withLock { cachedAgents?.find { it.id == id } }
?.let { return@safeApiCall it }
agentsApi.getAgent(id)
}
}
@ -201,7 +204,7 @@ class AgentRepositoryImpl(
}
}
private fun invalidateCache() {
cachedAgents = null
private suspend fun invalidateCache() {
cacheMutex.withLock { cachedAgents = null }
}
}

21
core/data/src/commonMain/kotlin/com/garfiec/librechat/core/data/repository/PresetRepositoryImpl.kt
View file

@ -4,28 +4,31 @@ import com.garfiec.librechat.core.common.result.Result
import com.garfiec.librechat.core.common.result.safeApiCall
import com.garfiec.librechat.core.model.Preset
import com.garfiec.librechat.core.network.api.PresetsApi
import kotlin.concurrent.Volatile
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
class PresetRepositoryImpl(
private val presetsApi: PresetsApi,
) : PresetRepository {
@Volatile
private val cacheMutex = Mutex()
private var cachedPresets: List<Preset>? = null
override suspend fun getAll(): Result<List<Preset>> {
return safeApiCall {
if (cachedPresets != null) return@safeApiCall cachedPresets!!
val presets = presetsApi.getPresets()
cachedPresets = presets
presets
cacheMutex.withLock {
cachedPresets?.let { return@withLock it }
val presets = presetsApi.getPresets()
cachedPresets = presets
presets
}
}
}
override suspend fun create(preset: Preset): Result<Preset> {
return safeApiCall {
val created = presetsApi.createPreset(preset)
cachedPresets = null
cacheMutex.withLock { cachedPresets = null }
created
}
}
@ -33,7 +36,7 @@ class PresetRepositoryImpl(
override suspend fun update(preset: Preset): Result<Preset> {
return safeApiCall {
val updated = presetsApi.updatePreset(preset)
cachedPresets = null
cacheMutex.withLock { cachedPresets = null }
updated
}
}
@ -41,7 +44,7 @@ class PresetRepositoryImpl(
override suspend fun delete(presetId: String): Result<Unit> {
return safeApiCall {
presetsApi.deletePreset(presetId)
cachedPresets = null
cacheMutex.withLock { cachedPresets = null }
}
}
}