android/okhttp
1
0
Fork 0
mirror of https://github.com/square/okhttp.git synced 2026-01-12 10:23:16 +03:00
Code Releases Activity

Don't start the reader thread until after the connection preface.

Browse Source 
This is slightly more work than ideal because our tests don't bother with the
connection preface. That makes the tests both simpler, and further from reality.
The workaround is a package-private method to keep the tests working as they
are currently.

Closes https://github.com/square/okhttp/issues/2469
This commit is contained in:
jwilson
2016-04-10 12:51:38 -04:00
parent e5b7409664
commit 8996934748
6 changed files with 47 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
mockwebserver/src/main/java/okhttp3/internal/framed/FramedServer.java
View File

@@ -70,7 +70,7 @@ public final class FramedServer extends FramedConnection.Listener {
.protocol(protocol)
.listener(this)
.build();
framedConnection.sendConnectionPreface();
framedConnection.start();
} catch (IOException e) {
logger.log(Level.INFO, "FramedServer connection failure: " + e);
Util.closeQuietly(socket);

1
mockwebserver/src/main/java/okhttp3/mockwebserver/MockWebServer.java
View File

@@ -449,6 +449,7 @@ public final class MockWebServer implements TestRule {
.protocol(protocol)
.listener(framedSocketListener)
.build();
framedConnection.start();
openFramedConnections.add(framedConnection);
openClientSockets.remove(socket);
return;

16
okhttp-tests/src/test/java/okhttp3/internal/framed/Http2ConnectionTest.java
View File

@@ -376,7 +376,9 @@ public final class Http2ConnectionTest {
// play it back
FramedConnection connection = connectionBuilder(peer, HTTP_2)
.pushObserver(observer).build();
.pushObserver(observer)
.build();
connection.start(false);
FramedStream client = connection.newStream(headerEntries("b", "banana"), false, true);
assertEquals(-1, client.getSource().read(new Buffer(), 1));
@@ -399,6 +401,7 @@ public final class Http2ConnectionTest {
// play it back
FramedConnection connection = connectionBuilder(peer, HTTP_2).build();
connection.start(false);
connection.newStream(headerEntries("b", "banana"), false, true);
// verify the peer received what was expected
@@ -423,8 +426,10 @@ public final class Http2ConnectionTest {
peer.play();
// play it back
connectionBuilder(peer, HTTP_2)
.pushObserver(PushObserver.CANCEL).build();
FramedConnection connection = connectionBuilder(peer, HTTP_2)
.pushObserver(PushObserver.CANCEL)
.build();
connection.start(false);
// verify the peer received what was expected
MockSpdyPeer.InFrame rstStream = peer.takeFrame();
@@ -452,6 +457,7 @@ public final class Http2ConnectionTest {
.pushObserver(IGNORE)
.protocol(HTTP_2.getProtocol())
.build();
connection.start(false);
socket.shutdownOutput();
try {
connection.newStream(headerEntries("a", longString), false, true);
@@ -488,7 +494,9 @@ public final class Http2ConnectionTest {
}
private FramedConnection connection(MockSpdyPeer peer, Variant variant) throws IOException {
return connectionBuilder(peer, variant).build();
FramedConnection connection = connectionBuilder(peer, variant).build();
connection.start(false);
return connection;
}
private FramedConnection.Builder connectionBuilder(MockSpdyPeer peer, Variant variant)

17
okhttp-tests/src/test/java/okhttp3/internal/framed/Spdy3ConnectionTest.java
View File

@@ -162,10 +162,11 @@ public final class Spdy3ConnectionTest {
stream.reply(headerEntries("b", "banana"), true);
}
};
new FramedConnection.Builder(true)
FramedConnection connection = new FramedConnection.Builder(true)
.socket(peer.openSocket())
.listener(handler)
.build();
connection.start(false);
// verify the peer received what was expected
MockSpdyPeer.InFrame reply = peer.takeFrame();
@@ -192,7 +193,10 @@ public final class Spdy3ConnectionTest {
}
};
connectionBuilder(peer, SPDY3).listener(listener).build();
FramedConnection connection = connectionBuilder(peer, SPDY3)
.listener(listener)
.build();
connection.start(false);
// verify the peer received what was expected
MockSpdyPeer.InFrame reply = peer.takeFrame();
@@ -282,6 +286,7 @@ public final class Spdy3ConnectionTest {
FramedConnection connection = connectionBuilder(peer, SPDY3)
.listener(listener)
.build();
connection.start(false);
peer.takeFrame(); // Guarantees that the peer Settings frame has been processed.
synchronized (connection) {
@@ -639,10 +644,11 @@ public final class Spdy3ConnectionTest {
stream.reply(headerEntries("c", "cola"), true);
}
};
new FramedConnection.Builder(true)
FramedConnection connection = new FramedConnection.Builder(true)
.socket(peer.openSocket())
.listener(listener)
.build();
connection.start(false);
// verify the peer received what was expected
MockSpdyPeer.InFrame reply = peer.takeFrame();
@@ -1340,6 +1346,7 @@ public final class Spdy3ConnectionTest {
.socket(socket)
.protocol(SPDY3.getProtocol())
.build();
connection.start(false);
socket.shutdownOutput();
try {
connection.newStream(headerEntries("a", longString), false, true);
@@ -1360,7 +1367,9 @@ public final class Spdy3ConnectionTest {
}
private FramedConnection connection(MockSpdyPeer peer, Variant variant) throws IOException {
return connectionBuilder(peer, variant).build();
FramedConnection connection = connectionBuilder(peer, variant).build();
connection.start(false);
return connection;
}
private FramedConnection.Builder connectionBuilder(MockSpdyPeer peer, Variant variant)

28
okhttp/src/main/java/okhttp3/internal/framed/FramedConnection.java
View File

@@ -170,7 +170,6 @@ public final class FramedConnection implements Closeable {
frameWriter = variant.newWriter(builder.sink, client);
readerRunnable = new Reader(variant.newReader(builder.source, client));
new Thread(readerRunnable).start(); // Not a daemon thread.
}
/** The protocol as selected using ALPN. */
@@ -501,16 +500,27 @@ public final class FramedConnection implements Closeable {
}
/**
* Sends a connection header if the current variant requires it. This should be called after
* {@link Builder#build} for all new connections.
* Sends any initial frames and starts reading frames from the remote peer. This should be called
* after {@link Builder#build} for all new connections.
*/
public void sendConnectionPreface() throws IOException {
frameWriter.connectionPreface();
frameWriter.settings(okHttpSettings);
int windowSize = okHttpSettings.getInitialWindowSize(Settings.DEFAULT_INITIAL_WINDOW_SIZE);
if (windowSize != Settings.DEFAULT_INITIAL_WINDOW_SIZE) {
frameWriter.windowUpdate(0, windowSize - Settings.DEFAULT_INITIAL_WINDOW_SIZE);
public void start() throws IOException {
start(true);
}
/**
* @param sendConnectionPreface true to send connection preface frames. This should always be true
* except for in tests that don't check for a connection preface.
*/
void start(boolean sendConnectionPreface) throws IOException {
if (sendConnectionPreface) {
frameWriter.connectionPreface();
frameWriter.settings(okHttpSettings);
int windowSize = okHttpSettings.getInitialWindowSize(Settings.DEFAULT_INITIAL_WINDOW_SIZE);
if (windowSize != Settings.DEFAULT_INITIAL_WINDOW_SIZE) {
frameWriter.windowUpdate(0, windowSize - Settings.DEFAULT_INITIAL_WINDOW_SIZE);
}
}
new Thread(readerRunnable).start(); // Not a daemon thread.
}
/** Merges {@code settings} into this peer's settings and sends them to the remote peer. */

2
okhttp/src/main/java/okhttp3/internal/io/RealConnection.java
View File

@@ -159,7 +159,7 @@ public final class RealConnection extends FramedConnection.Listener implements C
.protocol(protocol)
.listener(this)
.build();
framedConnection.sendConnectionPreface();
framedConnection.start();
// Only assign the framed connection once the preface has been sent successfully.
this.allocationLimit = framedConnection.maxConcurrentStreams();