Skip to content

Commit f9084ff

Browse files
authored
Fix race condition in plugin resolution during disconnect (#6269)
* Update `DependencyResolverTest` to verify error handling when dependency resolution races with disconnection. * Prevent race conditions during disconnects in `ChatClient`.
1 parent cdccfd9 commit f9084ff

2 files changed

Lines changed: 52 additions & 3 deletions

File tree

stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,7 @@ internal constructor(
354354
*
355355
* @see [Plugin]
356356
*/
357+
@Volatile
357358
@InternalStreamChatApi
358359
public var plugins: List<Plugin> = emptyList()
359360

@@ -400,12 +401,16 @@ internal constructor(
400401
@Suppress("ThrowsCount")
401402
internal inline fun <reified P : DependencyResolver, reified T : Any> resolvePluginDependency(): T {
402403
StreamLog.v(TAG) { "[resolvePluginDependency] P: ${P::class.simpleName}, T: ${T::class.simpleName}" }
404+
// Snapshot plugins BEFORE checking initializationState to avoid a race with disconnect().
405+
// disconnect() sets initializationState to NOT_INITIALIZED before clearing plugins,
406+
// so if we snapshot plugins first and then see COMPLETE, the snapshot is guaranteed valid.
407+
val currentPlugins = plugins
403408
val initState = awaitInitializationState(RESOLVE_DEPENDENCY_TIMEOUT)
404409
if (initState != InitializationState.COMPLETE) {
405410
StreamLog.e(TAG) { "[resolvePluginDependency] failed (initializationState is not COMPLETE): $initState " }
406411
throw IllegalStateException("ChatClient::connectUser() must be called before resolving any dependency")
407412
}
408-
val resolver = plugins.find { plugin ->
413+
val resolver = currentPlugins.find { plugin ->
409414
plugin is P
410415
} ?: throw IllegalStateException(
411416
"Plugin '${P::class.qualifiedName}' was not found. Did you init it within ChatClient?",
@@ -1570,9 +1575,9 @@ internal constructor(
15701575

15711576
notifications.onLogout()
15721577
// Set initializationState to NOT_INITIALIZED BEFORE clearing plugins to prevent race condition.
1573-
// This ensures the StatePlugin extension methods don't access the plugin during disconnect.
1578+
// resolvePluginDependency() snapshots plugins before checking state, so if it sees COMPLETE
1579+
// here, the snapshot is guaranteed to still contain the plugins.
15741580
mutableClientState.setInitializationState(InitializationState.NOT_INITIALIZED)
1575-
15761581
plugins.forEach { it.onUserDisconnected() }
15771582
plugins = emptyList()
15781583
userStateService.onLogout()

stream-chat-android-client/src/test/java/io/getstream/chat/android/client/DependencyResolverTest.kt

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@ import io.getstream.chat.android.models.NoOpMessageTransformer
3030
import io.getstream.chat.android.models.NoOpUserTransformer
3131
import io.getstream.chat.android.models.User
3232
import io.getstream.chat.android.test.TestCoroutineExtension
33+
import kotlinx.coroutines.awaitCancellation
34+
import kotlinx.coroutines.flow.FlowCollector
3335
import kotlinx.coroutines.flow.MutableStateFlow
36+
import kotlinx.coroutines.flow.StateFlow
3437
import kotlinx.coroutines.test.TestResult
3538
import kotlinx.coroutines.test.runTest
3639
import org.amshove.kluent.invoking
@@ -44,6 +47,7 @@ import org.junit.jupiter.params.provider.Arguments
4447
import org.junit.jupiter.params.provider.MethodSource
4548
import org.mockito.kotlin.mock
4649
import org.mockito.kotlin.whenever
50+
import java.util.concurrent.atomic.AtomicBoolean
4751
import kotlin.reflect.KClass
4852

4953
public class DependencyResolverTest {
@@ -129,6 +133,22 @@ public class DependencyResolverTest {
129133
fResult `should be` expectedDependency
130134
}
131135

136+
@Test
137+
public fun `Should resolve dependency when plugins are cleared during resolution`(): TestResult = runTest {
138+
val expectedDependency = SomeDependency()
139+
val fixture = Fixture()
140+
.with(PluginDependency(mapOf(SomeDependency::class to expectedDependency)))
141+
142+
val client = fixture.get()
143+
144+
val racingFlow = DisconnectSimulatingStateFlow(client)
145+
whenever(fixture.mutableClientState.initializationState).thenReturn(racingFlow)
146+
147+
val result = client.resolveDependency<PluginDependency, SomeDependency>()
148+
149+
result `should be` expectedDependency
150+
}
151+
132152
public companion object {
133153

134154
@JvmField
@@ -219,4 +239,28 @@ public class DependencyResolverTest {
219239
}
220240

221241
private class SomeDependency
242+
243+
private class DisconnectSimulatingStateFlow(
244+
private val client: ChatClient,
245+
) : StateFlow<InitializationState> {
246+
247+
private val disconnected = AtomicBoolean(false)
248+
249+
override val value: InitializationState
250+
get() {
251+
if (disconnected.compareAndSet(false, true)) {
252+
client.plugins = emptyList()
253+
return InitializationState.COMPLETE
254+
}
255+
return InitializationState.NOT_INITIALIZED
256+
}
257+
258+
override val replayCache: List<InitializationState>
259+
get() = listOf(InitializationState.COMPLETE)
260+
261+
override suspend fun collect(collector: FlowCollector<InitializationState>): Nothing {
262+
collector.emit(InitializationState.COMPLETE)
263+
awaitCancellation()
264+
}
265+
}
222266
}

0 commit comments

Comments
 (0)