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..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 @@ -549,22 +549,44 @@ 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 { - 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. + // + // 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 = cycleRegionUrlProvider + engine.join(connectUrl, token, options, roomOptions) + } catch (e: Exception) { + e.rethrowIfCancellationSignal() + + nextUrl = cycleRegionUrlProvider?.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 { + cycleRegionUrlProvider?.clearAttemptedRegions() } ensureActive() 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..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. @@ -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 = """{