Skip to content

Commit

Permalink
Read if needed on ProxyHandler's handshake. Fixes netty#5933.
Browse files Browse the repository at this point in the history
Motivation:

When auto-read is disabled and no reads are issued by a user, ProxyHandler will stall the connection on the proxy handshake phase waiting for the first response from a server (that was never read).

Modifications:

Read if needed when very first handshake message is send by ProxyHandler.

Result:

Proxy handshake now succeeds no matter if auto-read disabled or enabled. Without the fix, the new test is failing on master.
  • Loading branch information
vkostyukov authored and normanmaurer committed Oct 30, 2016
1 parent 9cfa467 commit 23f033a
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ public void run() {
if (initialMessage != null) {
sendToProxyServer(initialMessage);
}

readIfNeeded(ctx);
}

/**
Expand Down Expand Up @@ -384,9 +386,7 @@ public final void channelReadComplete(ChannelHandlerContext ctx) throws Exceptio
if (suppressChannelReadComplete) {
suppressChannelReadComplete = false;

if (!ctx.channel().config().isAutoRead()) {
ctx.read();
}
readIfNeeded(ctx);
} else {
ctx.fireChannelReadComplete();
}
Expand All @@ -412,6 +412,12 @@ public final void flush(ChannelHandlerContext ctx) throws Exception {
}
}

private static void readIfNeeded(ChannelHandlerContext ctx) {
if (!ctx.channel().config().isAutoRead()) {
ctx.read();
}
}

private void writePendingWrites() {
if (pendingWrites != null) {
pendingWrites.removeAndWriteAll();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import io.netty.channel.ChannelHandler;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInitializer;
import io.netty.channel.ChannelOption;
import io.netty.channel.ChannelPipeline;
import io.netty.channel.EventLoopGroup;
import io.netty.channel.SimpleChannelInboundHandler;
Expand Down Expand Up @@ -61,6 +62,7 @@
import java.util.Queue;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ThreadLocalRandom;
import java.util.concurrent.TimeUnit;

import static org.hamcrest.CoreMatchers.*;
Expand Down Expand Up @@ -363,9 +365,16 @@ private static final class SuccessTestHandler extends SimpleChannelInboundHandle
final Queue<Throwable> exceptions = new LinkedBlockingQueue<Throwable>();
volatile int eventCount;

private static void readIfNeeded(ChannelHandlerContext ctx) {
if (!ctx.channel().config().isAutoRead()) {
ctx.read();
}
}

@Override
public void channelActive(ChannelHandlerContext ctx) throws Exception {
ctx.writeAndFlush(Unpooled.copiedBuffer("A\n", CharsetUtil.US_ASCII));
readIfNeeded(ctx);
}

@Override
Expand All @@ -378,6 +387,7 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exc
// ProxyHandlers in the pipeline. Therefore, we send the 'B' message only on the first event.
ctx.writeAndFlush(Unpooled.copiedBuffer("B\n", CharsetUtil.US_ASCII));
}
readIfNeeded(ctx);
}
}

Expand All @@ -388,6 +398,7 @@ protected void channelRead0(ChannelHandlerContext ctx, Object msg) throws Except
if ("2".equals(str)) {
ctx.writeAndFlush(Unpooled.copiedBuffer("C\n", CharsetUtil.US_ASCII));
}
readIfNeeded(ctx);
}

@Override
Expand Down Expand Up @@ -523,6 +534,7 @@ protected void test() throws Exception {
Bootstrap b = new Bootstrap();
b.group(group);
b.channel(NioSocketChannel.class);
b.option(ChannelOption.AUTO_READ, ThreadLocalRandom.current().nextBoolean());
b.resolver(NoopAddressResolverGroup.INSTANCE);
b.handler(new ChannelInitializer<SocketChannel>() {
@Override
Expand Down

0 comments on commit 23f033a

Please sign in to comment.