From 150f97388e82e74816878ec2d829e2c4962026df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20J=C3=B6nsson?= Date: Wed, 24 Jun 2026 21:57:53 +0200 Subject: [PATCH] Retire pruned leaked connections so they can't be reused When a response body is abandoned (never closed), the GC-based leak detector in RealConnectionPool.pruneAndGetAllocationCount removes the leaked call's reference but, since #7456, no longer marks the connection noNewExchanges. A cleanup pass prunes every connection yet evicts at most one, so a leaked connection that isn't the evicted one stays idle and eligible for reuse while its socket still holds the abandoned response's unread bytes. The next call to reuse it reads those bytes as its status line, throwing "ProtocolException: Unexpected status line" and stapling one response's body onto another. 4.x marked the connection noNewExchanges here; this restores that line so any pruned leaked connection is retired. Fixes #9240 --- .../internal/connection/RealConnectionPool.kt | 6 +++++ .../internal/connection/ConnectionPoolTest.kt | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt index 2bf7da124c50..73ccdd3b42b3 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RealConnectionPool.kt @@ -311,6 +311,12 @@ class RealConnectionPool internal constructor( references.removeAt(i) + // A leaked connection may still hold the abandoned response's unread bytes on its socket, so it + // must not be handed to a new exchange. Retire it here rather than relying on eviction: a single + // cleanup pass prunes every connection but evicts at most one, so any leaked connection that + // isn't the one evicted would otherwise stay eligible for reuse and corrupt the next exchange. + connection.noNewExchanges = true + // If this was the last allocation, the connection is eligible for immediate eviction. if (references.isEmpty()) { connection.idleAtNs = now - keepAliveDurationNs diff --git a/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/ConnectionPoolTest.kt b/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/ConnectionPoolTest.kt index 8a676efeec48..85ca3f751e5a 100644 --- a/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/ConnectionPoolTest.kt +++ b/okhttp/src/jvmTest/kotlin/okhttp3/internal/connection/ConnectionPoolTest.kt @@ -178,6 +178,29 @@ class ConnectionPoolTest { assertThat(c1.noNewExchanges).isTrue() } + @Test fun prunedLeakedConnectionIsRetiredEvenWhenNotEvicted() { + val pool = factory.newConnectionPool() + val poolApi = ConnectionPool(pool) + + // Two pooled connections to the same address, each with an allocation the caller leaks. + val c1 = factory.newConnection(pool, routeA1, 0L) + val c2 = factory.newConnection(pool, routeA1, 0L) + allocateAndLeakAllocation(poolApi, c1) + allocateAndLeakAllocation(poolApi, c2) + + awaitGarbageCollection() + + // A single cleanup pass prunes the leaked allocation on both connections but evicts at most one. + pool.closeConnections(100L) + + // Any connection still in the pool must be retired. A leaked connection may hold the abandoned + // response's unread bytes, and reusing it would corrupt an unrelated exchange. + for (connection in listOf(c1, c2)) { + if (connection.socket().isClosed) continue // Evicted, so it can't be reused. + assertThat(connection.noNewExchanges).isTrue() + } + } + @Test fun interruptStopsThread() { val taskRunnerThreads = mutableListOf() val taskRunner =