HTTP/2 HelloWorld Client Example Bug

Motivation:
The HTTP/2 helloworld client example has 2 bugs:
1. HttpResponseHandler has a map which is accessed from multiple threads, but the map is not thread safe.
2. Requests are flushed and maybe completely written and the responses may be received/processed by Netty before an element is inserted into the HttpResponseHandler map. This may result in an 'unexpected message' error even though the message has actually been sent.

Modifications:
- HttpResponseHandler should use a thread safe map
- Http2Client shouldn't flush until entries are added to the HttpResponseHandler map

Result:
Fixes https://github.com/netty/netty/issues/6165.
This commit is contained in:
Scott Mitchell 2017-01-07 12:10:01 -08:00
parent ec3d077e0d
commit d771526f8c
2 changed files with 11 additions and 9 deletions

View File

@ -15,7 +15,6 @@
package io.netty.example.http2.helloworld.client;
import io.netty.bootstrap.Bootstrap;
import io.netty.buffer.Unpooled;
import io.netty.channel.Channel;
import io.netty.channel.ChannelOption;
import io.netty.channel.EventLoopGroup;
@ -44,6 +43,7 @@ import io.netty.util.CharsetUtil;
import java.util.concurrent.TimeUnit;
import static io.netty.buffer.Unpooled.wrappedBuffer;
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;
@ -118,20 +118,20 @@ public final class Http2Client {
request.headers().add(HttpConversionUtil.ExtensionHeaderNames.SCHEME.text(), scheme.name());
request.headers().add(HttpHeaderNames.ACCEPT_ENCODING, HttpHeaderValues.GZIP);
request.headers().add(HttpHeaderNames.ACCEPT_ENCODING, HttpHeaderValues.DEFLATE);
responseHandler.put(streamId, channel.writeAndFlush(request), channel.newPromise());
responseHandler.put(streamId, channel.write(request), channel.newPromise());
streamId += 2;
}
if (URL2 != null) {
// Create a simple POST request with a body.
FullHttpRequest request = new DefaultFullHttpRequest(HTTP_1_1, POST, URL2,
Unpooled.copiedBuffer(URL2DATA.getBytes(CharsetUtil.UTF_8)));
wrappedBuffer(URL2DATA.getBytes(CharsetUtil.UTF_8)));
request.headers().add(HttpHeaderNames.HOST, hostName);
request.headers().add(HttpConversionUtil.ExtensionHeaderNames.SCHEME.text(), scheme.name());
request.headers().add(HttpHeaderNames.ACCEPT_ENCODING, HttpHeaderValues.GZIP);
request.headers().add(HttpHeaderNames.ACCEPT_ENCODING, HttpHeaderValues.DEFLATE);
responseHandler.put(streamId, channel.writeAndFlush(request), channel.newPromise());
streamId += 2;
responseHandler.put(streamId, channel.write(request), channel.newPromise());
}
channel.flush();
responseHandler.awaitResponses(5, TimeUnit.SECONDS);
System.out.println("Finished HTTP/2 request(s)");

View File

@ -22,12 +22,12 @@ import io.netty.channel.SimpleChannelInboundHandler;
import io.netty.handler.codec.http.FullHttpResponse;
import io.netty.handler.codec.http2.HttpConversionUtil;
import io.netty.util.CharsetUtil;
import io.netty.util.internal.PlatformDependent;
import java.util.AbstractMap.SimpleEntry;
import java.util.Iterator;
import java.util.Map;
import java.util.Map.Entry;
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.concurrent.TimeUnit;
/**
@ -35,10 +35,12 @@ import java.util.concurrent.TimeUnit;
*/
public class HttpResponseHandler extends SimpleChannelInboundHandler<FullHttpResponse> {
private SortedMap<Integer, Entry<ChannelFuture, ChannelPromise>> streamidPromiseMap;
private Map<Integer, Entry<ChannelFuture, ChannelPromise>> streamidPromiseMap;
public HttpResponseHandler() {
streamidPromiseMap = new TreeMap<Integer, Entry<ChannelFuture, ChannelPromise>>();
// Use a concurrent map because we add and iterate from the main thread (just for the purposes of the example),
// but Netty also does a get on the map when messages are received in a EventLoop thread.
streamidPromiseMap = PlatformDependent.newConcurrentHashMap();
}
/**