From 75e87acee237f68a483694a66b96494e42eb32e8 Mon Sep 17 00:00:00 2001 From: shijing xian Date: Mon, 21 Sep 2026 15:19:13 -0700 Subject: [PATCH 1/4] Clear attempted regions when a connect failover cycle ends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `clearAttemptedRegions()` had a single call site — a successful reconnect — so the initial-connect failover loop in `Room.connect` never cleared the set it fills. The provider outlives one connect: `connect` reuses an existing provider for the same url and only creates one when it is null. Two consequences follow. After a successful connect, a region that failed transiently before another one succeeded stays excluded for the life of the provider, and a later failover skips it even once it has recovered. After an exhausted connect the set holds every region, so the next `connect` call resolves no region at all — it fails on the provided url and rethrows without trying a single fallback. Clear the set at both ends of the cycle, matching client-sdk-flutter, which resets on success, on exhaustion, and on a certificate-pinning failure. Co-Authored-By: Claude Opus 5 (1M context) --- .../main/java/io/livekit/android/room/Room.kt | 10 ++++++++ .../android/room/RegionUrlProviderTest.kt | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/livekit-android-sdk/src/main/java/io/livekit/android/room/Room.kt b/livekit-android-sdk/src/main/java/io/livekit/android/room/Room.kt index 12e0121e..22e50be8 100644 --- a/livekit-android-sdk/src/main/java/io/livekit/android/room/Room.kt +++ b/livekit-android-sdk/src/main/java/io/livekit/android/room/Room.kt @@ -562,11 +562,21 @@ constructor( if (nextUrl != null) { LKLog.d(e) { "Connection to $connectUrl failed, retrying with another region: $nextUrl" } } else { + // The attempted set belongs to this failover cycle, which ends here. The + // provider outlives it — `connect` reuses an existing one for the same + // url — so leaving the set full would make the next connect exhaust + // immediately, failing without trying a single region. + regionUrlProvider?.clearAttemptedRegions() throw e // rethrow since no more regions to try. } } } + // Connected, so the failover cycle is over. A region that failed transiently before + // this one succeeded must be eligible again next time, and only a successful + // *reconnect* cleared the set until now. + regionUrlProvider?.clearAttemptedRegions() + ensureActive() networkCallbackManager.registerCallback() if (options.audio) { diff --git a/livekit-android-test/src/test/java/io/livekit/android/room/RegionUrlProviderTest.kt b/livekit-android-test/src/test/java/io/livekit/android/room/RegionUrlProviderTest.kt index 30f60f7f..5511a6c8 100644 --- a/livekit-android-test/src/test/java/io/livekit/android/room/RegionUrlProviderTest.kt +++ b/livekit-android-test/src/test/java/io/livekit/android/room/RegionUrlProviderTest.kt @@ -79,6 +79,29 @@ class RegionUrlProviderTest : BaseTest() { // Check that only one request was needed. assertEquals(1, server.requestCount) } + + /** + * The attempted set scopes one failover cycle. Once it is cleared the whole region list is + * eligible again, so a region that failed transiently is retried on the next cycle rather + * than being skipped for the life of the provider. + */ + @Test + fun clearAttemptedRegionsMakesEveryRegionEligibleAgain() = runTest { + server = MockWebServer() + server.enqueue(MockResponse().setBody(regionResponse)) + val regionUrlProvider = RegionUrlProvider(server.url("").toUri(), "token", OkHttpClient.Builder().build(), Json { ignoreUnknownKeys = true }) + + assertEquals("https://regiona.livekit.cloud", regionUrlProvider.getNextBestRegionUrl()) + assertEquals("https://regionb.livekit.cloud", regionUrlProvider.getNextBestRegionUrl()) + + regionUrlProvider.clearAttemptedRegions() + + // Back to the best region, not the next unattempted one. + assertEquals("https://regiona.livekit.cloud", regionUrlProvider.getNextBestRegionUrl()) + + // Still served from the cached settings. + assertEquals(1, server.requestCount) + } } private val regionResponse = """{ From aac93e904a319881ea3fb55708dda58777ae4e97 Mon Sep 17 00:00:00 2001 From: Max Heimbrock <43608204+MaxHeimbrock@users.noreply.github.com> Date: Tue, 22 Sep 2026 10:27:57 +0200 Subject: [PATCH 2/4] Update copyright to pass kotlin spotless check --- .../test/java/io/livekit/android/room/RegionUrlProviderTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/livekit-android-test/src/test/java/io/livekit/android/room/RegionUrlProviderTest.kt b/livekit-android-test/src/test/java/io/livekit/android/room/RegionUrlProviderTest.kt index 5511a6c8..6cec4731 100644 --- a/livekit-android-test/src/test/java/io/livekit/android/room/RegionUrlProviderTest.kt +++ b/livekit-android-test/src/test/java/io/livekit/android/room/RegionUrlProviderTest.kt @@ -1,5 +1,5 @@ /* - * Copyright 2024 LiveKit, Inc. + * Copyright 2024-2026 LiveKit, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From cae3240da43cae8adc42de6268e69cb8fee17196 Mon Sep 17 00:00:00 2001 From: shijing xian Date: Tue, 22 Sep 2026 11:10:34 -0700 Subject: [PATCH 3/4] Clear attempted regions in finally, not per exit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `getNextBestRegionUrl` refreshes region settings once the cache expires and propagates request and decoding failures, so it can throw from inside the catch and carry the exception out of the loop — past both exit-specific clears. The cycle would then end with regions still marked attempted, and `connect` reuses the provider for the same url, so a later failover skips them. Scoping the loop with try/finally covers every exit at once: connected, out of regions, a settings refresh that threw, and cancellation. Co-Authored-By: Claude Opus 5 (1M context) --- .../main/java/io/livekit/android/room/Room.kt | 53 ++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/livekit-android-sdk/src/main/java/io/livekit/android/room/Room.kt b/livekit-android-sdk/src/main/java/io/livekit/android/room/Room.kt index 22e50be8..19060e6d 100644 --- a/livekit-android-sdk/src/main/java/io/livekit/android/room/Room.kt +++ b/livekit-android-sdk/src/main/java/io/livekit/android/room/Room.kt @@ -549,34 +549,39 @@ constructor( var nextUrl: String? = regionUrl ?: url regionUrl = null - while (nextUrl != null) { - val connectUrl = nextUrl - nextUrl = null - try { - engine.regionUrlProvider = regionUrlProvider - engine.join(connectUrl, token, options, roomOptions) - } catch (e: Exception) { - e.rethrowIfCancellationSignal() - - nextUrl = regionUrlProvider?.getNextBestRegionUrl() - if (nextUrl != null) { - LKLog.d(e) { "Connection to $connectUrl failed, retrying with another region: $nextUrl" } - } else { - // The attempted set belongs to this failover cycle, which ends here. The - // provider outlives it — `connect` reuses an existing one for the same - // url — so leaving the set full would make the next connect exhaust - // immediately, failing without trying a single region. - regionUrlProvider?.clearAttemptedRegions() - throw e // rethrow since no more regions to try. + // The attempted set scopes one failover cycle, and every way out of this loop ends + // that cycle: connected, out of regions, a region-settings refresh that threw, or + // cancellation. Clearing in `finally` covers all of them rather than one exit at a + // time — `getNextBestRegionUrl` refreshes settings after the cache expires and + // propagates request and decoding failures, so an exception can leave the loop from + // inside the catch. + // + // It has to be cleared somewhere, because the provider outlives the cycle: `connect` + // reuses an existing one for the same url. Regions left behind would be skipped by a + // later failover even once they recovered, and a set left full would make the next + // connect resolve no region at all. + try { + while (nextUrl != null) { + val connectUrl = nextUrl + nextUrl = null + try { + engine.regionUrlProvider = regionUrlProvider + engine.join(connectUrl, token, options, roomOptions) + } catch (e: Exception) { + e.rethrowIfCancellationSignal() + + nextUrl = regionUrlProvider?.getNextBestRegionUrl() + if (nextUrl != null) { + LKLog.d(e) { "Connection to $connectUrl failed, retrying with another region: $nextUrl" } + } else { + throw e // rethrow since no more regions to try. + } } } + } finally { + regionUrlProvider?.clearAttemptedRegions() } - // Connected, so the failover cycle is over. A region that failed transiently before - // this one succeeded must be eligible again next time, and only a successful - // *reconnect* cleared the set until now. - regionUrlProvider?.clearAttemptedRegions() - ensureActive() networkCallbackManager.registerCallback() if (options.audio) { From 2ef1a301620e1f96059d0d5ef7b5fd093dc9d14b Mon Sep 17 00:00:00 2001 From: shijing xian Date: Tue, 22 Sep 2026 11:42:54 -0700 Subject: [PATCH 4/4] Capture one region provider for the failover cycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `regionUrlProvider` is a mutable field and `prepareConnection` replaces it without holding `stateLock`, so reading it separately in the loop and in `finally` can touch two different instances: the engine keeps the one the cycle actually used while the clear lands on its replacement, leaving the engine's provider holding regions a later failover then skips. Read it once into a local and use that for the engine handoff, the next-region lookup, and the clear. Serializing provider installation against the connection state transition is the other half of that race and is left alone here — it changes `prepareConnection`'s locking, which is outside this fix. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/main/java/io/livekit/android/room/Room.kt | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/livekit-android-sdk/src/main/java/io/livekit/android/room/Room.kt b/livekit-android-sdk/src/main/java/io/livekit/android/room/Room.kt index 19060e6d..9c430859 100644 --- a/livekit-android-sdk/src/main/java/io/livekit/android/room/Room.kt +++ b/livekit-android-sdk/src/main/java/io/livekit/android/room/Room.kt @@ -560,17 +560,24 @@ constructor( // reuses an existing one for the same url. Regions left behind would be skipped by a // later failover even once they recovered, and a set left full would make the next // connect resolve no region at all. + // + // Captured once for the whole cycle rather than read from the field each time: + // `regionUrlProvider` is mutable and `prepareConnection` replaces it without holding + // `stateLock`, so re-reading could hand one instance to the engine and clear a + // different one — leaving the engine's provider holding regions it would then skip. + val cycleRegionUrlProvider = regionUrlProvider + try { while (nextUrl != null) { val connectUrl = nextUrl nextUrl = null try { - engine.regionUrlProvider = regionUrlProvider + engine.regionUrlProvider = cycleRegionUrlProvider engine.join(connectUrl, token, options, roomOptions) } catch (e: Exception) { e.rethrowIfCancellationSignal() - nextUrl = regionUrlProvider?.getNextBestRegionUrl() + nextUrl = cycleRegionUrlProvider?.getNextBestRegionUrl() if (nextUrl != null) { LKLog.d(e) { "Connection to $connectUrl failed, retrying with another region: $nextUrl" } } else { @@ -579,7 +586,7 @@ constructor( } } } finally { - regionUrlProvider?.clearAttemptedRegions() + cycleRegionUrlProvider?.clearAttemptedRegions() } ensureActive()