From 7f37a76f21baa806c10bb6548bcc4525b6ffac68 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sun, 4 Jan 2026 12:56:57 +0000 Subject: [PATCH 1/3] Avoid leaked connections being re-allocated Multiple leaked connections will only close one, so make sure it's not used for new exchanges until it's closed. --- .../internal/connection/RealConnectionPool.kt | 5 ++++ .../internal/connection/ConnectionPoolTest.kt | 29 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt index 85b334cd6bf1..87b22ab875b0 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt @@ -307,6 +307,11 @@ class RealConnectionPool internal constructor( // If this was the last allocation, the connection is eligible for immediate eviction. if (references.isEmpty()) { + // If we have cleared a leaked call reference, we must ensure this + // can't be reallocated even if multiple connections were leaked + // and only one is about to be closed + connection.noNewExchanges = true + connection.idleAtNs = now - keepAliveDurationNs return 0 } diff --git a/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/ConnectionPoolTest.kt b/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/ConnectionPoolTest.kt index 8a676efeec48..22e12a7249c8 100644 --- a/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/ConnectionPoolTest.kt +++ b/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/ConnectionPoolTest.kt @@ -20,6 +20,7 @@ import assertk.assertions.isEmpty import assertk.assertions.isEqualTo import assertk.assertions.isFalse import assertk.assertions.isNotEmpty +import assertk.assertions.isNotSameInstanceAs import assertk.assertions.isTrue import okhttp3.ConnectionPool import okhttp3.FakeRoutePlanner @@ -178,6 +179,34 @@ class ConnectionPoolTest { assertThat(c1.noNewExchanges).isTrue() } + @Test fun multipleLeakedAllocations() { + val pool = factory.newConnectionPool() + val poolApi = ConnectionPool(pool) + val c1 = factory.newConnection(pool, routeA1, 0L) + allocateAndLeakAllocation(poolApi, c1) + val c2 = factory.newConnection(pool, routeA1, 0L) + allocateAndLeakAllocation(poolApi, c2) + awaitGarbageCollection() + assertThat(pool.closeConnections(100L)).isEqualTo(0L) + + assertThat(c1.calls).isEmpty() + assertThat(c1.noNewExchanges).isTrue() + + assertThat(c2.calls).isEmpty() + assertThat(c2.noNewExchanges).isTrue() + + val client = + OkHttpClient + .Builder() + .connectionPool(poolApi) + .build() + val call = client.newCall(Request(c1.route().address.url)) as RealCall + val c3 = pool.callAcquirePooledConnection(false, c1.route().address, call, null, false) + + assertThat(c3).isNotSameInstanceAs(c1) + assertThat(c3).isNotSameInstanceAs(c2) + } + @Test fun interruptStopsThread() { val taskRunnerThreads = mutableListOf() val taskRunner = From 2d1832efcbd57371efd241ec478ce7505a84e083 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sun, 4 Jan 2026 12:59:19 +0000 Subject: [PATCH 2/3] Avoid leaked connections being re-allocated Multiple leaked connections will only close one, so make sure it's not used for new exchanges until it's closed. --- .../okhttp3/internal/connection/RealConnectionPool.kt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt index 87b22ab875b0..3f04aae2c28e 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt @@ -307,10 +307,12 @@ class RealConnectionPool internal constructor( // If this was the last allocation, the connection is eligible for immediate eviction. if (references.isEmpty()) { - // If we have cleared a leaked call reference, we must ensure this - // can't be reallocated even if multiple connections were leaked - // and only one is about to be closed - connection.noNewExchanges = true + if (!connection.isMultiplexed) { + // If we have cleared a leaked call reference for a HTTP/1.1 connection, + // we must ensure this can't be reallocated even if multiple connections were leaked + // and only one is about to be closed + connection.noNewExchanges = true + } connection.idleAtNs = now - keepAliveDurationNs return 0 From a148b6cedd26413c946ab8d9f3193ff6920ba22c Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sun, 4 Jan 2026 13:07:38 +0000 Subject: [PATCH 3/3] better comments --- .../kotlin/okhttp3/internal/connection/RealConnectionPool.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt index 3f04aae2c28e..3894b7c4c3a1 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt @@ -307,11 +307,14 @@ class RealConnectionPool internal constructor( // If this was the last allocation, the connection is eligible for immediate eviction. if (references.isEmpty()) { - if (!connection.isMultiplexed) { + if (!connection.isMultiplexed && !connection.noNewExchanges) { // If we have cleared a leaked call reference for a HTTP/1.1 connection, // we must ensure this can't be reallocated even if multiple connections were leaked // and only one is about to be closed connection.noNewExchanges = true + + // Can't notify while holding lock + // connection.connectionListener.noNewExchanges(connection) } connection.idleAtNs = now - keepAliveDurationNs