1
0
mirror of https://github.com/square/okhttp.git synced 2026-01-24 04:02:07 +03:00

Fix a race condition in Connection.isExpired().

This was causing some SPDY connections to be evicted early,
and preventing connection reuse.

The spdyConnectionReuse test becomes flaky with the
fix that makes MockWebServer use different sequenceNumbers
for different requests on the same socket. With the fix
the test is no longer flaky. The old test wasn't good enough
to detect what it wanted to detect!

This race has existed in the code since we added more
aggressive time-based expiration in January 2013. In my
basic tests the race impacted ~20% of connections on a
desktop VM. It may have been more on mobile.
This commit is contained in:
jwilson
2013-09-28 15:43:31 -04:00
parent 081258a266
commit 29ab48bf0d
4 changed files with 10 additions and 6 deletions

View File

@@ -633,6 +633,7 @@ public final class MockWebServer {
private class SpdySocketHandler implements IncomingStreamHandler {
private final Socket socket;
private final Transport transport;
private final AtomicInteger sequenceNumber = new AtomicInteger();
private SpdySocketHandler(Socket socket, Transport transport) {
this.socket = socket;
@@ -684,7 +685,7 @@ public final class MockWebServer {
String requestLine = method + ' ' + path + ' ' + version;
List<Integer> chunkSizes = Collections.emptyList(); // No chunked encoding for SPDY.
return new RecordedRequest(requestLine, httpHeaders, chunkSizes, bodyOut.size(),
bodyOut.toByteArray(), 0, socket);
bodyOut.toByteArray(), sequenceNumber.getAndIncrement(), socket);
}
private void writeResponse(SpdyStream stream, MockResponse response) throws IOException {

View File

@@ -122,15 +122,18 @@ public final class SpdyConnection implements Closeable {
}
private synchronized void setIdle(boolean value) {
idleStartTimeNs = value ? System.nanoTime() : 0L;
idleStartTimeNs = value ? System.nanoTime() : Long.MAX_VALUE;
}
/** Returns true if this connection is idle. */
public synchronized boolean isIdle() {
return idleStartTimeNs != 0L;
return idleStartTimeNs != Long.MAX_VALUE;
}
/** Returns the time in ns when this connection became idle or 0L if connection is not idle. */
/**
* Returns the time in ns when this connection became idle or Long.MAX_VALUE
* if connection is not idle.
*/
public synchronized long getIdleStartTimeNs() {
return idleStartTimeNs;
}

View File

@@ -244,7 +244,7 @@ public final class Connection implements Closeable {
* {@code keepAliveDurationNs}.
*/
public boolean isExpired(long keepAliveDurationNs) {
return isIdle() && System.nanoTime() - getIdleStartTimeNs() > keepAliveDurationNs;
return getIdleStartTimeNs() < System.nanoTime() - keepAliveDurationNs;
}
/**

View File

@@ -142,7 +142,7 @@ public final class HttpOverSpdyTest {
assertEquals("DEF", readAscii(connection1.getInputStream(), 3));
assertEquals("JKL", readAscii(connection2.getInputStream(), 3));
assertEquals(0, server.takeRequest().getSequenceNumber());
assertEquals(0, server.takeRequest().getSequenceNumber());
assertEquals(1, server.takeRequest().getSequenceNumber());
}
@Test public void gzippedResponseBody() throws Exception {