Skip to content

Commit

Permalink
Recover from a coalesced connection that immediately goes unhealthy.
Browse files Browse the repository at this point in the history
  • Loading branch information
dave-r12 committed Aug 11, 2019
1 parent 10abb26 commit e9bffc4
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 0 deletions.
96 changes: 96 additions & 0 deletions okhttp-tests/src/test/java/okhttp3/ConnectionCoalescingTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import javax.net.ssl.HostnameVerifier;
Expand Down Expand Up @@ -107,6 +108,101 @@ public final class ConnectionCoalescingTest {
assertThat(client.connectionPool().connectionCount()).isEqualTo(1);
}

/**
* This is an extraordinary test case. Here's what it's trying to simulate.
* - 2 requests happen concurrently to a host that can be coalesced onto a single connection.
* - Both request discover no existing connection. They both make a connection.
* - The first request "wins the race".
* - The second request discovers it "lost the race" and closes the connection it just opened.
* - The second request uses the coalesced connection from request1.
* - The coalesced connection is violently closed after servicing the first request.
* - The second request discovers the coalesced connection is unhealthy just after acquiring it.
*/
@Test public void coalescedConnectionDestroyedAfterAcquire() throws Exception {
server.enqueue(new MockResponse().setResponseCode(200));
server.enqueue(new MockResponse().setResponseCode(200));

dns.set("san.com", Dns.SYSTEM.lookup(server.getHostName()).subList(0, 1));
HttpUrl sanUrl = url.newBuilder().host("san.com").build();

CountDownLatch request2ConnectStart = new CountDownLatch(1);
CountDownLatch request1ConnectionAcquired = new CountDownLatch(1);
CountDownLatch request2ConnectionAcquired = new CountDownLatch(1);
CountDownLatch request1SocketClosed = new CountDownLatch(1);
EventListener listener1 = new EventListener() {
@Override public void connectStart(Call call, InetSocketAddress inetSocketAddress,
Proxy proxy) {
try {
request2ConnectStart.await();
} catch (InterruptedException e) {
throw new AssertionError(e);
}
}

@Override public void connectionAcquired(Call call, Connection connection) {
request1ConnectionAcquired.countDown();
}
};

EventListener request2Listener = new EventListener() {
@Override public void connectStart(Call call, InetSocketAddress inetSocketAddress,
Proxy proxy) {
request2ConnectStart.countDown();
try {
request1ConnectionAcquired.await();
} catch (InterruptedException e) {
throw new AssertionError(e);
}
}

@Override public void connectionAcquired(Call call, Connection connection) {
request2ConnectionAcquired.countDown();
try {
request1SocketClosed.await();
} catch (InterruptedException e) {
throw new AssertionError(e);
}
}
};

// Get a reference to the connection so we can violently destroy it.
AtomicReference<Connection> connection = new AtomicReference<>();
OkHttpClient client1 = client.newBuilder()
.addNetworkInterceptor(chain -> {
connection.set(chain.connection());
return chain.proceed(chain.request());
})
.eventListener(listener1)
.build();

Request request = new Request.Builder().url(sanUrl).build();
Call call1 = client1.newCall(request);
call1.enqueue(new Callback() {
@Override public void onResponse(Call call, Response response) throws IOException {
try {
request2ConnectionAcquired.await();
} catch (InterruptedException e) {
throw new AssertionError(e);
}
assert200Http2Response(response, "san.com");
connection.get().socket().close();
request1SocketClosed.countDown();
}

@Override public void onFailure(Call call, IOException e) {
fail();
}
});

OkHttpClient client2 = client.newBuilder()
.eventListener(request2Listener)
.build();
Call call2 = client2.newCall(request);
Response response = call2.execute();

assert200Http2Response(response, "san.com");
}

/**
* Test connecting to an alternative host then common name, although only subject alternative
* names are used if present no special consideration of common name.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,10 @@ private RealConnection findConnection(int connectTimeout, int readTimeout, int w
result.noNewExchanges = true;
socket = result.socket();
result = transmitter.connection;

// It's possible for us to obtain a coalesced connection that is immediately unhealthy. In
// that case we will retry the route we just successfully connected with.
nextRouteToTry = selectedRoute;
} else {
connectionPool.put(result);
transmitter.acquireConnectionNoEvents(result);
Expand Down

0 comments on commit e9bffc4

Please sign in to comment.