andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

HTTP/2 Unit Test race condition

Browse Source 
Motivation:
The Http2ConnectionRoundtripTest.noMoreStreamIdsShouldSendGoAway unit test had a race condition where it would sometimes receive a SETINGS_ACK message that was not anticipated. This caused the test to fail because of bad test code.

Modifications:
The bad unit test should be updated to handle the message exchange for a good connection setup, and then the GO_AWAY frame.

Result:
Http2ConnectionRoundtripTest.noMoreStreamIdsShouldSendGoAway should no longer sporadically fail.
This commit is contained in:
Scott Mitchell 2014-12-08 18:20:25 -05:00
parent 047176bc3f
commit 7f9fb95702
6 changed files with 52 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java
View File

@ -32,7 +32,6 @@ public final class Http2CodecUtil {
private static final byte[] CONNECTION_PREFACE = "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n".getBytes(UTF_8);
private static final byte[] EMPTY_PING = new byte[8];
private static final IgnoreSettingsHandler IGNORE_SETTINGS_HANDLER = new IgnoreSettingsHandler();
public static final int CONNECTION_STREAM_ID = 0;
public static final int HTTP_UPGRADE_STREAM_ID = 1;
@ -129,15 +128,6 @@ public final class Http2CodecUtil {
};
}
/**
* Creates a new {@link ChannelHandler} that does nothing but ignore inbound settings frames.
* This is a useful utility to avoid verbose logging output for pipelines that don't handle
* settings frames directly.
*/
public static ChannelHandler ignoreSettingsHandler() {
return IGNORE_SETTINGS_HANDLER;
}
/**
* Iteratively looks through the causaility chain for the given exception and returns the first
* {@link Http2Exception} or {@code null} if none.
@ -216,21 +206,6 @@ public final class Http2CodecUtil {
throw cause;
}
/**
* A{@link ChannelHandler} that does nothing but ignore inbound settings frames. This is a
* useful utility to avoid verbose logging output for pipelines that don't handle settings
* frames directly.
*/
@ChannelHandler.Sharable
private static class IgnoreSettingsHandler extends ChannelHandlerAdapter {
@Override
public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
if (!(msg instanceof Http2Settings)) {
super.channelRead(ctx, msg);
}
}
}
private Http2CodecUtil() {
}
}

25
codec-http2/src/test/java/io/netty/handler/codec/http2/DataCompressionHttp2Test.java
View File

@ -82,8 +82,9 @@ public class DataCompressionHttp2Test {
private Bootstrap cb;
private Channel serverChannel;
private Channel clientChannel;
private volatile CountDownLatch serverLatch;
private volatile CountDownLatch clientLatch;
private CountDownLatch serverLatch;
private CountDownLatch clientLatch;
private CountDownLatch clientSettingsAckLatch;
private Http2Connection serverConnection;
private Http2Connection clientConnection;
private ByteArrayOutputStream serverOut;
@ -111,7 +112,7 @@ public class DataCompressionHttp2Test {
@Test
public void justHeadersNoData() throws Exception {
bootstrapEnv(1, 0, 1);
bootstrapEnv(1, 1, 0, 1);
final Http2Headers headers = new DefaultHttp2Headers().method(GET).path(PATH)
.set(HttpHeaderNames.CONTENT_ENCODING, HttpHeaderValues.GZIP);
@ -135,7 +136,7 @@ public class DataCompressionHttp2Test {
public void gzipEncodingSingleEmptyMessage() throws Exception {
final String text = "";
final ByteBuf data = Unpooled.copiedBuffer(text.getBytes());
bootstrapEnv(1, data.readableBytes(), 1);
bootstrapEnv(1, 1, data.readableBytes(), 1);
try {
final Http2Headers headers = new DefaultHttp2Headers().method(POST).path(PATH)
.set(HttpHeaderNames.CONTENT_ENCODING, HttpHeaderValues.GZIP);
@ -164,7 +165,7 @@ public class DataCompressionHttp2Test {
public void gzipEncodingSingleMessage() throws Exception {
final String text = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbccccccccccccccccccccccc";
final ByteBuf data = Unpooled.copiedBuffer(text.getBytes());
bootstrapEnv(1, data.readableBytes(), 1);
bootstrapEnv(1, 1, data.readableBytes(), 1);
try {
final Http2Headers headers = new DefaultHttp2Headers().method(POST).path(PATH)
.set(HttpHeaderNames.CONTENT_ENCODING, HttpHeaderValues.GZIP);
@ -195,7 +196,7 @@ public class DataCompressionHttp2Test {
final String text2 = "dddddddddddddddddddeeeeeeeeeeeeeeeeeeeffffffffffffffffffff";
final ByteBuf data1 = Unpooled.copiedBuffer(text1.getBytes());
final ByteBuf data2 = Unpooled.copiedBuffer(text2.getBytes());
bootstrapEnv(1, data1.readableBytes() + data2.readableBytes(), 1);
bootstrapEnv(1, 1, data1.readableBytes() + data2.readableBytes(), 1);
try {
final Http2Headers headers = new DefaultHttp2Headers().method(POST).path(PATH)
.set(HttpHeaderNames.CONTENT_ENCODING, HttpHeaderValues.GZIP);
@ -228,7 +229,7 @@ public class DataCompressionHttp2Test {
final int BUFFER_SIZE = 1 << 12;
final byte[] bytes = new byte[BUFFER_SIZE];
new Random().nextBytes(bytes);
bootstrapEnv(1, BUFFER_SIZE, 1);
bootstrapEnv(1, 1, BUFFER_SIZE, 1);
final ByteBuf data = Unpooled.wrappedBuffer(bytes);
try {
final Http2Headers headers = new DefaultHttp2Headers().method(POST).path(PATH)
@ -255,10 +256,12 @@ public class DataCompressionHttp2Test {
}
}
private void bootstrapEnv(int serverHalfClosedCount, int serverOutSize, int clientCount) throws Exception {
private void bootstrapEnv(int serverHalfClosedCount, int clientSettingsAckLatchCount,
int serverOutSize, int clientCount) throws Exception {
serverOut = new ByteArrayOutputStream(serverOutSize);
serverLatch = new CountDownLatch(serverHalfClosedCount);
clientLatch = new CountDownLatch(clientCount);
clientSettingsAckLatch = new CountDownLatch(clientSettingsAckLatchCount);
sb = new ServerBootstrap();
cb = new Bootstrap();
@ -304,7 +307,6 @@ public class DataCompressionHttp2Test {
new CompressorHttp2ConnectionEncoder.Builder().connection(
serverConnection).frameWriter(writer));
p.addLast(connectionHandler);
p.addLast(Http2CodecUtil.ignoreSettingsHandler());
serverChannelLatch.countDown();
}
});
@ -315,7 +317,8 @@ public class DataCompressionHttp2Test {
@Override
protected void initChannel(Channel ch) throws Exception {
ChannelPipeline p = ch.pipeline();
FrameCountDown clientFrameCountDown = new FrameCountDown(clientListener, clientLatch);
FrameCountDown clientFrameCountDown = new FrameCountDown(clientListener,
clientSettingsAckLatch, clientLatch);
Http2FrameWriter writer = new DefaultHttp2FrameWriter();
Http2ConnectionHandler connectionHandler =
new Http2ConnectionHandler(new DefaultHttp2ConnectionDecoder.Builder()
@ -328,7 +331,6 @@ public class DataCompressionHttp2Test {
clientConnection).frameWriter(writer));
clientEncoder = connectionHandler.encoder();
p.addLast(connectionHandler);
p.addLast(Http2CodecUtil.ignoreSettingsHandler());
}
});
@ -342,6 +344,7 @@ public class DataCompressionHttp2Test {
}
private void awaitServer() throws Exception {
assertTrue(clientSettingsAckLatch.await(5, SECONDS));
assertTrue(serverLatch.await(5, SECONDS));
serverOut.flush();
}

37
codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionRoundtripTest.java
View File

@ -87,6 +87,7 @@ public class Http2ConnectionRoundtripTest {
private Channel clientChannel;
private FrameCountDown serverFrameCountDown;
private CountDownLatch requestLatch;
private CountDownLatch serverSettingsAckLatch;
private CountDownLatch dataLatch;
private CountDownLatch trailersLatch;
@ -110,7 +111,7 @@ public class Http2ConnectionRoundtripTest {
@Test
public void http2ExceptionInPipelineShouldCloseConnection() throws Exception {
bootstrapEnv(1, 1, 1);
bootstrapEnv(1, 1, 1, 1);
// Create a latch to track when the close occurs.
final CountDownLatch closeLatch = new CountDownLatch(1);
@ -132,6 +133,7 @@ public class Http2ConnectionRoundtripTest {
});
// Wait for the server to create the stream.
assertTrue(serverSettingsAckLatch.await(5, SECONDS));
assertTrue(requestLatch.await(5, SECONDS));
// Add a handler that will immediately throw an exception.
@ -154,7 +156,7 @@ public class Http2ConnectionRoundtripTest {
any(ChannelHandlerContext.class), eq(3), eq(headers), eq(0), eq((short) 16),
eq(false), eq(0), eq(false));
bootstrapEnv(1, 1, 1);
bootstrapEnv(1, 0, 1, 1);
// Create a latch to track when the close occurs.
final CountDownLatch closeLatch = new CountDownLatch(1);
@ -175,6 +177,7 @@ public class Http2ConnectionRoundtripTest {
});
// Wait for the server to create the stream.
assertTrue(serverSettingsAckLatch.await(5, SECONDS));
assertTrue(requestLatch.await(5, SECONDS));
// Wait for the close to occur.
@ -184,7 +187,7 @@ public class Http2ConnectionRoundtripTest {
@Test
public void nonHttp2ExceptionInPipelineShouldNotCloseConnection() throws Exception {
bootstrapEnv(1, 1, 1);
bootstrapEnv(1, 1, 1, 1);
// Create a latch to track when the close occurs.
final CountDownLatch closeLatch = new CountDownLatch(1);
@ -206,6 +209,7 @@ public class Http2ConnectionRoundtripTest {
});
// Wait for the server to create the stream.
assertTrue(serverSettingsAckLatch.await(5, SECONDS));
assertTrue(requestLatch.await(5, SECONDS));
// Add a handler that will immediately throw an exception.
@ -223,7 +227,7 @@ public class Http2ConnectionRoundtripTest {
@Test
public void noMoreStreamIdsShouldSendGoAway() throws Exception {
bootstrapEnv(1, 3, 1);
bootstrapEnv(1, 1, 3, 1);
// Create a single stream by sending a HEADERS frame to the server.
final Http2Headers headers = dummyHeaders();
@ -232,12 +236,19 @@ public class Http2ConnectionRoundtripTest {
public void run() {
http2Client.encoder().writeHeaders(ctx(), 3, headers, 0, (short) 16, false, 0,
true, newPromise());
}
});
assertTrue(serverSettingsAckLatch.await(5, SECONDS));
runInChannel(clientChannel, new Http2Runnable() {
@Override
public void run() {
http2Client.encoder().writeHeaders(ctx(), Integer.MAX_VALUE + 1, headers, 0, (short) 16, false, 0,
true, newPromise());
}
});
// Wait for the server to create the stream.
assertTrue(requestLatch.await(5, SECONDS));
verify(serverListener).onGoAwayRead(any(ChannelHandlerContext.class), eq(0),
eq(Http2Error.PROTOCOL_ERROR.code()), any(ByteBuf.class));
@ -267,7 +278,7 @@ public class Http2ConnectionRoundtripTest {
any(ByteBuf.class), eq(0), anyBoolean());
try {
// Initialize the data latch based on the number of bytes expected.
bootstrapEnv(length, 2, 1);
bootstrapEnv(length, 1, 2, 1);
// Create the stream and send all of the data at once.
runInChannel(clientChannel, new Http2Runnable() {
@ -284,6 +295,7 @@ public class Http2ConnectionRoundtripTest {
});
// Wait for the trailers to be received.
assertTrue(serverSettingsAckLatch.await(5, SECONDS));
assertTrue(trailersLatch.await(5, SECONDS));
// Verify that headers and trailers were received.
@ -347,7 +359,7 @@ public class Http2ConnectionRoundtripTest {
}).when(serverListener).onDataRead(any(ChannelHandlerContext.class), anyInt(),
any(ByteBuf.class), anyInt(), anyBoolean());
try {
bootstrapEnv(numStreams * length, numStreams * 4, numStreams);
bootstrapEnv(numStreams * length, 1, numStreams * 4, numStreams);
runInChannel(clientChannel, new Http2Runnable() {
@Override
public void run() {
@ -367,6 +379,7 @@ public class Http2ConnectionRoundtripTest {
}
});
// Wait for all frames to be received.
assertTrue(serverSettingsAckLatch.await(60, SECONDS));
assertTrue(trailersLatch.await(60, SECONDS));
verify(serverListener, times(numStreams)).onHeadersRead(any(ChannelHandlerContext.class), anyInt(),
eq(headers), eq(0), eq((short) 16), eq(false), eq(0), eq(false));
@ -388,8 +401,10 @@ public class Http2ConnectionRoundtripTest {
}
}
private void bootstrapEnv(int dataCountDown, int requestCountDown, int trailersCountDown) throws Exception {
private void bootstrapEnv(int dataCountDown, int settingsAckCount,
int requestCountDown, int trailersCountDown) throws Exception {
requestLatch = new CountDownLatch(requestCountDown);
serverSettingsAckLatch = new CountDownLatch(settingsAckCount);
dataLatch = new CountDownLatch(dataCountDown);
trailersLatch = new CountDownLatch(trailersCountDown);
sb = new ServerBootstrap();
@ -402,10 +417,9 @@ public class Http2ConnectionRoundtripTest {
protected void initChannel(Channel ch) throws Exception {
ChannelPipeline p = ch.pipeline();
serverFrameCountDown =
new FrameCountDown(serverListener, requestLatch, dataLatch,
trailersLatch);
new FrameCountDown(serverListener, serverSettingsAckLatch,
requestLatch, dataLatch, trailersLatch);
p.addLast(new Http2ConnectionHandler(true, serverFrameCountDown));
p.addLast(Http2CodecUtil.ignoreSettingsHandler());
}
});
@ -416,7 +430,6 @@ public class Http2ConnectionRoundtripTest {
protected void initChannel(Channel ch) throws Exception {
ChannelPipeline p = ch.pipeline();
p.addLast(new Http2ConnectionHandler(false, clientListener));
p.addLast(Http2CodecUtil.ignoreSettingsHandler());
}
});

2
codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameRoundtripTest.java
View File

@ -363,7 +363,6 @@ public class Http2FrameRoundtripTest {
ChannelPipeline p = ch.pipeline();
serverAdapter = new Http2TestUtil.FrameAdapter(serverListener, requestLatch);
p.addLast("reader", serverAdapter);
p.addLast(Http2CodecUtil.ignoreSettingsHandler());
}
});
@ -374,7 +373,6 @@ public class Http2FrameRoundtripTest {
protected void initChannel(Channel ch) throws Exception {
ChannelPipeline p = ch.pipeline();
p.addLast("reader", new Http2TestUtil.FrameAdapter(null, null));
p.addLast(Http2CodecUtil.ignoreSettingsHandler());
}
});

14
codec-http2/src/test/java/io/netty/handler/codec/http2/Http2TestUtil.java
View File

@ -262,21 +262,19 @@ final class Http2TestUtil {
static class FrameCountDown implements Http2FrameListener {
private final Http2FrameListener listener;
private final CountDownLatch messageLatch;
private final CountDownLatch settingsAckLatch;
private final CountDownLatch dataLatch;
private final CountDownLatch trailersLatch;
FrameCountDown(Http2FrameListener listener, CountDownLatch messageLatch) {
this(listener, messageLatch, null);
FrameCountDown(Http2FrameListener listener, CountDownLatch settingsAckLatch, CountDownLatch messageLatch) {
this(listener, settingsAckLatch, messageLatch, null, null);
}
FrameCountDown(Http2FrameListener listener, CountDownLatch messageLatch, CountDownLatch dataLatch) {
this(listener, messageLatch, dataLatch, null);
}
FrameCountDown(Http2FrameListener listener, CountDownLatch messageLatch,
FrameCountDown(Http2FrameListener listener, CountDownLatch settingsAckLatch, CountDownLatch messageLatch,
CountDownLatch dataLatch, CountDownLatch trailersLatch) {
this.listener = listener;
this.messageLatch = messageLatch;
this.settingsAckLatch = settingsAckLatch;
this.dataLatch = dataLatch;
this.trailersLatch = trailersLatch;
}
@ -331,7 +329,7 @@ final class Http2TestUtil {
@Override
public void onSettingsAckRead(ChannelHandlerContext ctx) throws Http2Exception {
listener.onSettingsAckRead(ctx);
messageLatch.countDown();
settingsAckLatch.countDown();
}
@Override

15
codec-http2/src/test/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandlerTest.java
View File

@ -17,7 +17,6 @@ package io.netty.handler.codec.http2;
import static io.netty.handler.codec.http.HttpMethod.GET;
import static io.netty.handler.codec.http.HttpMethod.POST;
import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
import static io.netty.handler.codec.http2.Http2CodecUtil.ignoreSettingsHandler;
import static io.netty.handler.codec.http2.Http2TestUtil.as;
import static io.netty.util.CharsetUtil.UTF_8;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
@ -83,7 +82,8 @@ public class HttpToHttp2ConnectionHandlerTest {
private Bootstrap cb;
private Channel serverChannel;
private Channel clientChannel;
private volatile CountDownLatch requestLatch;
private CountDownLatch requestLatch;
private CountDownLatch serverSettingsAckLatch;
private FrameCountDown serverFrameCountDown;
@Before
@ -104,7 +104,7 @@ public class HttpToHttp2ConnectionHandlerTest {
@Test
public void testJustHeadersRequest() throws Exception {
bootstrapEnv(3);
bootstrapEnv(2, 1);
final FullHttpRequest request = new DefaultFullHttpRequest(HTTP_1_1, GET, "/example");
final HttpHeaders httpHeaders = request.headers();
httpHeaders.setInt(HttpUtil.ExtensionHeaderNames.STREAM_ID.text(), 5);
@ -147,7 +147,7 @@ public class HttpToHttp2ConnectionHandlerTest {
}
}).when(serverListener).onDataRead(any(ChannelHandlerContext.class), eq(3),
any(ByteBuf.class), eq(0), eq(true));
bootstrapEnv(4);
bootstrapEnv(3, 1);
final FullHttpRequest request = new DefaultFullHttpRequest(HTTP_1_1, POST, "/example",
Unpooled.copiedBuffer(text, UTF_8));
final HttpHeaders httpHeaders = request.headers();
@ -177,8 +177,9 @@ public class HttpToHttp2ConnectionHandlerTest {
assertEquals(0, request.refCnt());
}
private void bootstrapEnv(int requestCountDown) throws Exception {
private void bootstrapEnv(int requestCountDown, int serverSettingsAckCount) throws Exception {
requestLatch = new CountDownLatch(requestCountDown);
serverSettingsAckLatch = new CountDownLatch(serverSettingsAckCount);
sb = new ServerBootstrap();
cb = new Bootstrap();
@ -189,9 +190,8 @@ public class HttpToHttp2ConnectionHandlerTest {
@Override
protected void initChannel(Channel ch) throws Exception {
ChannelPipeline p = ch.pipeline();
serverFrameCountDown = new FrameCountDown(serverListener, requestLatch);
serverFrameCountDown = new FrameCountDown(serverListener, serverSettingsAckLatch, requestLatch);
p.addLast(new HttpToHttp2ConnectionHandler(true, serverFrameCountDown));
p.addLast(ignoreSettingsHandler());
}
});
@ -202,7 +202,6 @@ public class HttpToHttp2ConnectionHandlerTest {
protected void initChannel(Channel ch) throws Exception {
ChannelPipeline p = ch.pipeline();
p.addLast(new HttpToHttp2ConnectionHandler(false, clientListener));
p.addLast(ignoreSettingsHandler());
}
});