Cap connection-failure retries at 21 attempts

okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/http/RetryAndFollowUpInterceptor.kt

@@ -76,6 +76,7 @@ class RetryAndFollowUpInterceptor : Interceptor {
           val isRecoverable = recover(e, call, chain, request)
           call.eventListener.retryDecision(call, e, isRecoverable)
           if (!isRecoverable) throw e.withSuppressed(recoveredFailures)
+          if (recoveredFailures.size > MAX_RECOVERED_FAILURES) throw e.withSuppressed(recoveredFailures)
           recoveredFailures += e
           newRoutePlanner = false
           continue
@@ -353,5 +354,13 @@ class RetryAndFollowUpInterceptor : Interceptor {
      * curl, and wget follow 20; Safari follows 16; and HTTP/1.0 recommends 5.
      */
     private const val MAX_FOLLOW_UPS = 20
+
+    /**
+     * How many connection failures should we retry before giving up? Without a cap, a route
+     * selector that keeps yielding usable routes (or keeps cycling through an exhausted set)
+     * lets this loop retry indefinitely, growing [recoveredFailures] without bound and repeatedly
+     * contending for locks a stuck attempt is still holding.
+     */
+    private const val MAX_RECOVERED_FAILURES = 20
   }
 }

okhttp/src/jvmTest/kotlin/okhttp3/CallTest.kt

@@ -1316,6 +1316,39 @@ open class CallTest {
     recoverWhenRetryOnConnectionFailureIsTrue()
   }
 
+  @Test
+  fun doesNotRetryConnectionFailuresUnboundedly() {
+    // Provide more usable routes (25) than the cap allows. The cap check runs before the current
+    // failure is appended to recoveredFailures (so the thrown exception is never suppressed by
+    // itself), which means the throw fires one request later than MAX_RECOVERED_FAILURES retries
+    // plus the original attempt would suggest: 22, not 21. Mirrors DoubleInetAddressDns's
+    // technique of resolving the same real address repeatedly to guarantee fallback routes.
+    // Without the cap, recover() would keep finding a usable route and the call would consume
+    // closer to all 25 queued failures before route exhaustion finally stopped it. Asserting the
+    // exact request count, not just that some IOException was eventually thrown, is what actually
+    // proves the cap fired rather than route exhaustion coincidentally doing the same job.
+    val dispatcher = QueueDispatcher()
+    repeat(25) {
+      dispatcher.enqueue(MockResponse.Builder().onResponseStart(CloseSocket()).build())
+    }
+    server.dispatcher = dispatcher
+    client =
+      client
+        .newBuilder()
+        .dns(
+          object : Dns {
+            override fun lookup(hostname: String): List<InetAddress> {
+              val address = Dns.SYSTEM.lookup(hostname)[0]
+              return List(25) { address }
+            }
+          },
+        ).build()
+    assertFailsWith<IOException> {
+      client.newCall(Request.Builder().url(server.url("/")).build()).execute()
+    }
+    assertThat(server.requestCount).isEqualTo(22)
+  }
+
   @Test
   fun noRecoverWhenRetryOnConnectionFailureIsFalse() {
     server.enqueue(MockResponse(body = "seed connection pool"))