-
Notifications
You must be signed in to change notification settings - Fork 58
Single-flight Call.join to stop concurrent-join race #1764
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 3 commits
6013e68
3ba89a4
e3a5c08
dec065e
0c47d95
edebe8e
e009429
1c9226c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,7 @@ | |
| import io.getstream.video.android.core.call.SfuConnectFailureCause | ||
| import io.getstream.video.android.core.call.SfuConnectionResult | ||
| import io.getstream.video.android.core.model.toIceServer | ||
| import io.getstream.video.android.core.utils.StreamRefCountedSingleFlightProcessor | ||
| import kotlinx.coroutines.CoroutineScope | ||
| import kotlinx.coroutines.delay | ||
| import kotlinx.coroutines.flow.first | ||
|
|
@@ -67,8 +68,33 @@ | |
| ) { | ||
| private val logger by taggedLogger("Call:JoinCoordinator:$type:$id") | ||
|
|
||
| /** | ||
| * Coalesces concurrent [join] calls into one attempt on the call [scope]. | ||
| * | ||
| * Without this, overlapping joins each build an [RtcSession] while reusing | ||
| * [CallSessionManager.sessionId], which leaves SFU-evicted zombies that fail every RPC | ||
| * with PARTICIPANT_NOT_FOUND. Checking [CallSessionManager.session] is not enough — it | ||
| * is only set after the coordinator round-trip. | ||
| * | ||
| * Coalescing the whole [join] also keeps once-per-join work once-only: JoinInitiated / | ||
| * MediaDevicePermission analytics, installing [CallState.callJoinInterceptor], resetting | ||
| * the leave guard, and moving to [RealtimeConnection.InProgress]. | ||
| * | ||
| * [StreamRefCountedSingleFlightProcessor] keeps the join alive when one waiter (e.g. an | ||
| * Activity) is cancelled while others still await, and cancels it when the last waiter | ||
| * leaves. | ||
| */ | ||
| private val joinFlight = StreamRefCountedSingleFlightProcessor(scope) | ||
|
|
||
| private fun isVideoEnabled(): Boolean = state.settings.value?.video?.enabled ?: false | ||
|
|
||
| /** | ||
| * Joins the call, coalescing concurrent callers into one in-flight execution (single-flight). | ||
| * | ||
| * The shared work runs on the call [scope]. Each caller still awaits on its own coroutine, | ||
| * so destroying one UI scope only drops that waiter; remaining waiters keep the join. | ||
| * Cancelling the last waiter cancels the shared job. | ||
| */ | ||
| suspend fun join( | ||
| create: Boolean = false, | ||
| createOptions: CreateCallOptions? = null, | ||
|
|
@@ -77,6 +103,42 @@ | |
| hintHighScaleLivestreamPublisher: Boolean? = null, | ||
| callJoinInterceptor: CallJoinInterceptor? = null, | ||
| ): Result<RtcSession> { | ||
| return joinFlight.run(JOIN_FLIGHT_KEY) { | ||
| executeJoin( | ||
| create, | ||
| createOptions, | ||
| ring, | ||
| notify, | ||
| hintHighScaleLivestreamPublisher, | ||
| callJoinInterceptor, | ||
| ) | ||
| }.fold( | ||
| onSuccess = { it }, | ||
| onFailure = { error -> | ||
| Failure( | ||
| Error.ThrowableError( | ||
| message = error.message ?: "Join single-flight failed", | ||
| cause = error, | ||
| ), | ||
| ) | ||
| }, | ||
| ) | ||
| } | ||
|
|
||
| private suspend fun executeJoin( | ||
| create: Boolean, | ||
| createOptions: CreateCallOptions?, | ||
| ring: Boolean, | ||
| notify: Boolean, | ||
| hintHighScaleLivestreamPublisher: Boolean?, | ||
| callJoinInterceptor: CallJoinInterceptor?, | ||
| ): Result<RtcSession> { | ||
| // Idempotent: subsequent join() while a session is already live returns that session | ||
| sessionManager.session.value?.let { existing -> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Telemetry deltas worth a deliberate call, since join reporting matters:
Suggest a distinct signal on the already-joined and coalesced paths instead of going quiet, plus a |
||
| logger.i { "[join] Call already joined — returning existing session" } | ||
| return Success(existing) | ||
| } | ||
|
|
||
| callAnalytics.joinAnalytics.onJoinFunctionStart() | ||
| callAnalytics.mediaPermissionObserver.mediaPermissionStatus() | ||
| logger.d { | ||
|
|
@@ -194,7 +256,7 @@ | |
| } | ||
|
|
||
| fun isPermanentError(error: Any): Boolean { | ||
| if (error is Error.ThrowableError) { | ||
|
Check warning on line 259 in stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/call/components/CallJoinCoordinator.kt
|
||
| if (error.message.contains("Unable to resolve host")) { | ||
| return false | ||
| } | ||
|
|
@@ -213,8 +275,9 @@ | |
| sessionManager.nonFastReconnectAttempts = 0 | ||
| sessionMonitor.cancelSfuObservers() | ||
|
|
||
| if (sessionManager.session.value != null) { | ||
| return Failure(Error.GenericError("Call $type:$id has already been joined")) | ||
| sessionManager.session.value?.let { existing -> | ||
| logger.i { "[joinInternal] Call already joined — returning existing session" } | ||
| return Success(existing) | ||
| } | ||
| logger.d { | ||
| "[joinInternal] #track; create: $create, ring: $ring, notify: $notify, createOptions: $createOptions" | ||
|
|
@@ -300,6 +363,7 @@ | |
| "[_join] Got terminal error while connecting to SFU. Error : $sfuConnectionResult" | ||
| } | ||
| sendJoinErrorAnalytics(sfuConnectionResult) | ||
| discardFailedSession(localSession) | ||
| return Failure( | ||
| Error.GenericError( | ||
| sfuConnectionResult.error.message ?: "RtcSession error occurred.", | ||
|
|
@@ -308,10 +372,11 @@ | |
| } | ||
| } | ||
|
|
||
| if (sfuConnectionResult.cause != SfuConnectFailureCause.TerminalSocketFailure) { | ||
|
Check warning on line 375 in stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/call/components/CallJoinCoordinator.kt
|
||
| if (!didReconnectSucceed()) { | ||
| logger.e { "[_join] Could not recover. Error : $sfuConnectionResult" } | ||
| sendJoinErrorAnalytics(sfuConnectionResult) | ||
| discardFailedSession(localSession) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| return Failure( | ||
| Error.GenericError( | ||
| sfuConnectionResult.error.message ?: "SFU connection failed", | ||
|
|
@@ -335,6 +400,28 @@ | |
| return Success(value = connectedSession) | ||
| } | ||
|
|
||
| /** | ||
| * Tears down every session left after a failed join connect. Clearing the reference | ||
| * alone is not enough: sockets and peer connections stay alive and keep issuing SFU | ||
| * RPCs for a participant that is gone, which the SFU answers with PARTICIPANT_NOT_FOUND. | ||
| * | ||
| * Recoverable failures may already have swapped in a replacement via [CallReconnector] | ||
| * before [didReconnectSucceed] settles as failed. That replacement is not useful once | ||
| * join is returning Failure — tear it down too so nothing live is left behind. | ||
| */ | ||
| private fun discardFailedSession(localSession: RtcSession) { | ||
| val active = sessionManager.session.value | ||
| logger.d { | ||
| "[joinInternal] Discarding session(s) after failed join connect " + | ||
| "(activeIsJoinSession=${active === localSession})" | ||
| } | ||
| sessionManager.setActiveSession(null) | ||
| if (active != null && active !== localSession) { | ||
| active.cleanup() | ||
| } | ||
| localSession.cleanup() | ||
| } | ||
|
|
||
| /** | ||
| * Reports the SFU WebSocket join failure to analytics. Only called from the join | ||
| * flow ([joinInternal]) so that reconnect-driven [RtcSession.connectInternal] failures | ||
|
|
@@ -366,4 +453,8 @@ | |
| logger.d { "[_join] Reconnect after recoverable connection failure settled on $terminal" } | ||
| return terminal is RealtimeConnection.Connected | ||
| } | ||
|
|
||
| private companion object { | ||
| const val JOIN_FLIGHT_KEY = "join" | ||
| } | ||
|
PratimMallick marked this conversation as resolved.
Outdated
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| /* | ||
| * Copyright (c) 2014-2026 Stream.io Inc. All rights reserved. | ||
| * | ||
| * Licensed under the Stream License; | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * https://github.com/GetStream/stream-video-android/blob/main/LICENSE | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package io.getstream.video.android.core.utils | ||
|
|
||
| // package io.getstream.android.core.internal.processing | ||
|
PratimMallick marked this conversation as resolved.
Outdated
|
||
|
|
||
| import kotlinx.coroutines.CancellationException | ||
| import kotlinx.coroutines.CoroutineScope | ||
| import kotlinx.coroutines.Deferred | ||
| import kotlinx.coroutines.NonCancellable | ||
| import kotlinx.coroutines.async | ||
| import kotlinx.coroutines.channels.ClosedSendChannelException | ||
| import kotlinx.coroutines.sync.Mutex | ||
| import kotlinx.coroutines.sync.withLock | ||
| import kotlinx.coroutines.withContext | ||
| import java.util.concurrent.atomic.AtomicBoolean | ||
|
|
||
| /** | ||
| * Single-flight that coalesces concurrent calls by key, runs the shared work on [scope], | ||
| * and tracks how many callers are still awaiting. | ||
| * | ||
| * Compared to [StreamSingleFlightProcessorImpl]: | ||
| * - Cancelling **one** waiter does **not** cancel the shared job (work is owned by [scope]). | ||
| * - Cancelling the **last** waiter **does** cancel the shared job (sole-caller cancel still | ||
| * aborts the operation). | ||
| * - [CancellationException] is rethrown to the cancelled waiter instead of being wrapped in | ||
| * [Result.failure]. | ||
| * | ||
| * Use this when the operation should survive Activity/ViewModel teardown of some waiters | ||
| * (e.g. screen handoff, UI join + call-scoped auto-join) but should not keep running after | ||
| * nobody is waiting — unless [scope] itself is cancelled (leave / call cleanup). | ||
| * | ||
| * Candidate for Stream Android Core v2 alongside [StreamSingleFlightProcessorImpl]. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Re-ran this at HEAD after On the KDoc though: Two things that point the same way: |
||
| */ | ||
| internal class StreamRefCountedSingleFlightProcessor( | ||
| private val scope: CoroutineScope, | ||
| ) { | ||
| private class Flight<T>( | ||
| val deferred: Deferred<Result<T>>, | ||
| var waiters: Int, | ||
| ) | ||
|
|
||
| private val mutex = Mutex() | ||
| private val flights = mutableMapOf<String, Flight<*>>() | ||
| private val closed = AtomicBoolean(false) | ||
|
|
||
| /** | ||
| * Runs [block] once for [key] while concurrent callers await the same result. | ||
| * | ||
| * Returns [Result.failure] with a [ClosedSendChannelException] if [stop] has already | ||
| * been called. [CancellationException] is still rethrown when this waiter (or the shared | ||
| * job, including last-waiter cancel) is cancelled. | ||
| */ | ||
| @Suppress("UNCHECKED_CAST") | ||
| suspend fun <T> run(key: String, block: suspend () -> T): Result<T> { | ||
|
Check failure on line 69 in stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/utils/StreamRefCountedSingleFlightProcessor.kt
|
||
|
PratimMallick marked this conversation as resolved.
|
||
| if (closed.get()) { | ||
| return Result.failure(ClosedSendChannelException("RefCountedSingleFlight is closed")) | ||
| } | ||
|
|
||
| val flight = mutex.withLock { | ||
| val running = flights[key]?.takeUnless { it.deferred.isCompleted } as Flight<T>? | ||
|
PratimMallick marked this conversation as resolved.
Outdated
|
||
| if (running != null) { | ||
| running.waiters++ | ||
| running | ||
| } else { | ||
| val deferred = scope.async { | ||
| try { | ||
| // Complete normally even when [block] fails so the scope does not see an | ||
| // uncaught child exception; waiters receive Result.failure after await. | ||
| try { | ||
| Result.success(block()) | ||
| } catch (ce: CancellationException) { | ||
| throw ce | ||
| } catch (t: Throwable) { | ||
| Result.failure(t) | ||
| } | ||
| } finally { | ||
| mutex.withLock { | ||
| if (flights[key]?.deferred === this@async) { | ||
| flights.remove(key) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Flight(deferred = deferred, waiters = 1).also { flights[key] = it } | ||
| } | ||
| } | ||
|
|
||
| var released = false | ||
| suspend fun releaseWaiter(cancelIfLast: Boolean) { | ||
| if (released) return | ||
| released = true | ||
| val shouldCancelShared = mutex.withLock { | ||
| flight.waiters = (flight.waiters - 1).coerceAtLeast(0) | ||
| cancelIfLast && flight.waiters == 0 && flight.deferred.isActive | ||
| } | ||
| if (shouldCancelShared) { | ||
|
PratimMallick marked this conversation as resolved.
Outdated
|
||
| flight.deferred.cancel() | ||
| } | ||
| } | ||
|
|
||
| return try { | ||
| flight.deferred.await() | ||
| } catch (ce: CancellationException) { | ||
| // NonCancellable: waiter bookkeeping must run while this coroutine is cancelling. | ||
| withContext(NonCancellable) { | ||
| releaseWaiter(cancelIfLast = true) | ||
| } | ||
| throw ce | ||
| } finally { | ||
| withContext(NonCancellable) { | ||
| releaseWaiter(cancelIfLast = false) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fun has(key: String): Boolean = flights.containsKey(key) | ||
|
|
||
| fun cancel(key: String): Result<Unit> = runCatching { | ||
| flights[key]?.deferred?.cancel() | ||
| } | ||
|
|
||
| fun clear(cancelRunning: Boolean): Result<Unit> = runCatching { | ||
| if (cancelRunning) { | ||
| flights.values.forEach { it.deferred.cancel() } | ||
| } | ||
| flights.clear() | ||
| } | ||
|
|
||
| fun stop(): Result<Unit> = runCatching { | ||
| if (closed.compareAndSet(false, true)) { | ||
| clear(cancelRunning = true).getOrThrow() | ||
| } | ||
|
PratimMallick marked this conversation as resolved.
|
||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.