fix: build each shared HTTP client once instead of racing to replace it

routedHttpClient() and webSocketClient() both did a plain check-then-set
on their AtomicReference: read, and if empty build a client and store it.
Two threads arriving together each saw an empty reference, each built a
full OkHttpClient, and the loser's client was dropped on the floor with
its connection pool and dispatcher threads already allocated. Nothing
closed it, so the leak lasted until the process died.

Moves construction inside a lock and re-checks the reference there, so
the second thread returns the first thread's client rather than building
its own. reset() takes the same lock, which is what makes the pairing
airtight: a build can no longer interleave with a reset and store a
client for the route that was just discarded. The fast path stays outside
the lock, so a warm client still costs a single volatile read.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Moe Hamade 2026-08-09 14:53:42 +03:00
parent 99bed510de
commit 000a8acdb9
2 changed files with 50 additions and 18 deletions

View File

@ -22,10 +22,13 @@ object OkHttpProvider {
private val httpClientRef = AtomicReference<RoutedClient?>(null)
private val wsClientRef = AtomicReference<OkHttpClient?>(null)
private val clientLock = Any()
fun reset() {
httpClientRef.set(null)
wsClientRef.set(null)
synchronized(clientLock) {
httpClientRef.set(null)
wsClientRef.set(null)
}
}
fun httpClient(): OkHttpClient = routedHttpClient().client
@ -38,26 +41,29 @@ object OkHttpProvider {
*/
fun routedHttpClient(): RoutedClient {
httpClientRef.get()?.let { return it }
val (builder, route) = baseBuilderForCurrentProxy()
val client = builder
.callTimeout(15, TimeUnit.SECONDS)
.connectTimeout(10, TimeUnit.SECONDS)
.readTimeout(15, TimeUnit.SECONDS)
.build()
val routedClient = RoutedClient(client, route)
httpClientRef.set(routedClient)
return routedClient
return synchronized(clientLock) {
httpClientRef.get() ?: run {
val (builder, route) = baseBuilderForCurrentProxy()
val client = builder
.callTimeout(15, TimeUnit.SECONDS)
.connectTimeout(10, TimeUnit.SECONDS)
.readTimeout(15, TimeUnit.SECONDS)
.build()
RoutedClient(client, route).also(httpClientRef::set)
}
}
}
fun webSocketClient(): OkHttpClient {
wsClientRef.get()?.let { return it }
val client = baseBuilderForCurrentProxy().first
.connectTimeout(10, TimeUnit.SECONDS)
.readTimeout(0, TimeUnit.SECONDS)
.writeTimeout(10, TimeUnit.SECONDS)
.build()
wsClientRef.set(client)
return client
return synchronized(clientLock) {
wsClientRef.get() ?: baseBuilderForCurrentProxy().first
.connectTimeout(10, TimeUnit.SECONDS)
.readTimeout(0, TimeUnit.SECONDS)
.writeTimeout(10, TimeUnit.SECONDS)
.build()
.also(wsClientRef::set)
}
}
private fun baseBuilderForCurrentProxy(): Pair<OkHttpClient.Builder, Route> {

View File

@ -0,0 +1,26 @@
package com.bitchat.android.net
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotSame
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
@RunWith(RobolectricTestRunner::class)
class OkHttpProviderTest {
@Test
fun `reset clears cached clients without changing the route`() {
OkHttpProvider.reset()
val cachedHttp = OkHttpProvider.routedHttpClient()
val cachedWebSocket = OkHttpProvider.webSocketClient()
OkHttpProvider.reset()
val rebuiltHttp = OkHttpProvider.routedHttpClient()
val rebuiltWebSocket = OkHttpProvider.webSocketClient()
assertEquals(cachedHttp.route, rebuiltHttp.route)
assertNotSame(cachedHttp.client, rebuiltHttp.client)
assertNotSame(cachedWebSocket, rebuiltWebSocket)
}
}