Permalink
Browse files

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 #6165.
1 parent ec3d077 commit d771526f8cfe262914ef40f0613e1e4b9e3a3aca @Scottmitch Scottmitch committed Jan 7, 2017
@@ -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 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 static void main(String[] args) throws Exception {
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)");
@@ -22,23 +22,25 @@
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;
/**
* Process {@link io.netty.handler.codec.http.FullHttpResponse} translated from HTTP/2 frames
*/
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();
}
/**

0 comments on commit d771526

Please sign in to comment.