From 000a8acdb98b22f556c5381b19cb5d293132dc29 Mon Sep 17 00:00:00 2001
From: Moe Hamade <69801237+moehamade@users.noreply.github.com>
Date: Sun, 9 Aug 2026 14:53:42 +0300
Subject: [PATCH] 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>
---
 .../com/bitchat/android/net/OkHttpProvider.kt | 42 +++++++++++--------
 .../bitchat/android/net/OkHttpProviderTest.kt | 26 ++++++++++++
 2 files changed, 50 insertions(+), 18 deletions(-)
 create mode 100644 app/src/test/kotlin/com/bitchat/android/net/OkHttpProviderTest.kt

diff --git a/app/src/main/java/com/bitchat/android/net/OkHttpProvider.kt b/app/src/main/java/com/bitchat/android/net/OkHttpProvider.kt
index 7f4d7c29..f9ad16e2 100644
--- a/app/src/main/java/com/bitchat/android/net/OkHttpProvider.kt
+++ b/app/src/main/java/com/bitchat/android/net/OkHttpProvider.kt
@@ -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> {
diff --git a/app/src/test/kotlin/com/bitchat/android/net/OkHttpProviderTest.kt b/app/src/test/kotlin/com/bitchat/android/net/OkHttpProviderTest.kt
new file mode 100644
index 00000000..95117435
--- /dev/null
+++ b/app/src/test/kotlin/com/bitchat/android/net/OkHttpProviderTest.kt
@@ -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)
+    }
+}