Skip to content

Commit bd09dcf

Browse files
authored
Pin the read count on both autoRead paths (#2304)
Motivation: #2302 stopped AsyncHttpClientHandler requesting a read that Netty's HeadContext already drives when autoRead is on. Nothing in the suite catches that duplicate coming back, since the connection works correctly either way Modification: Add AsyncHttpClientHandlerReadTest, an EmbeddedChannel test that counts reads at an outbound handler placed in front of the handler under test, so it sees both entry points. Covers channelActive and channelReadComplete with autoRead on, and channelReadComplete with it off. Result: One read per cycle with autoRead on, reads still driven with it off; without #2302 the count is 2.
1 parent 516804e commit bd09dcf

1 file changed

Lines changed: 98 additions & 0 deletions

File tree

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/*
2+
* Copyright (c) 2014-2026 AsyncHttpClient Project. All rights reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.asynchttpclient.netty.handler;
17+
18+
import io.netty.channel.Channel;
19+
import io.netty.channel.ChannelHandler;
20+
import io.netty.channel.ChannelHandlerContext;
21+
import io.netty.channel.ChannelOutboundHandlerAdapter;
22+
import io.netty.channel.embedded.EmbeddedChannel;
23+
import org.asynchttpclient.DefaultAsyncHttpClientConfig;
24+
import org.asynchttpclient.netty.NettyResponseFuture;
25+
import org.junit.jupiter.api.Test;
26+
27+
import java.util.concurrent.atomic.AtomicInteger;
28+
29+
import static org.junit.jupiter.api.Assertions.assertEquals;
30+
31+
/**
32+
* Guards the read behaviour of {@link AsyncHttpClientHandler#channelActive} and
33+
* {@link AsyncHttpClientHandler#channelReadComplete}.
34+
* <p>
35+
* Netty's {@code HeadContext} already calls {@code Channel#read()} after firing either event whenever
36+
* autoRead is on, so a read requested from the handler as well doubles the outbound traversal and
37+
* {@code doBeginRead} for every read cycle. That duplication is invisible to the functional suites — the
38+
* connection works either way — so it needs pinning here.
39+
* <p>
40+
* The counting handler goes in front of the handler under test so that it sees both entry points: the
41+
* {@code ctx.read()} the handler issues itself, and the {@code Channel#read()} that HeadContext drives in
42+
* from the tail. {@link EmbeddedChannel} fires channelActive while it registers and its {@code doBeginRead}
43+
* is a no-op, so counting in the pipeline is the only way to observe either.
44+
*/
45+
class AsyncHttpClientHandlerReadTest {
46+
47+
@Test
48+
void readsOncePerCycleWhenAutoReadIsOn() {
49+
AtomicInteger reads = new AtomicInteger();
50+
EmbeddedChannel channel = new EmbeddedChannel(readCounter(reads), handler());
51+
52+
// channelActive fired during registration; HeadContext requested the read, the handler must not have.
53+
assertEquals(1, reads.get());
54+
55+
reads.set(0);
56+
channel.pipeline().fireChannelReadComplete();
57+
assertEquals(1, reads.get());
58+
}
59+
60+
@Test
61+
void stillReadsWhenAutoReadIsOff() {
62+
AtomicInteger reads = new AtomicInteger();
63+
EmbeddedChannel channel = new EmbeddedChannel(readCounter(reads), handler());
64+
channel.config().setAutoRead(false);
65+
66+
// With autoRead off HeadContext requests nothing, so the handler is the only thing keeping the
67+
// connection from stalling.
68+
reads.set(0);
69+
channel.pipeline().fireChannelReadComplete();
70+
assertEquals(1, reads.get());
71+
}
72+
73+
private static ChannelHandler readCounter(AtomicInteger reads) {
74+
return new ChannelOutboundHandlerAdapter() {
75+
@Override
76+
public void read(ChannelHandlerContext ctx) {
77+
reads.incrementAndGet();
78+
ctx.read();
79+
}
80+
};
81+
}
82+
83+
private static AsyncHttpClientHandler handler() {
84+
return new AsyncHttpClientHandler(new DefaultAsyncHttpClientConfig.Builder().build(), null, null) {
85+
@Override
86+
public void handleRead(Channel channel, NettyResponseFuture<?> future, Object message) {
87+
}
88+
89+
@Override
90+
public void handleException(NettyResponseFuture<?> future, Throwable error) {
91+
}
92+
93+
@Override
94+
public void handleChannelInactive(NettyResponseFuture<?> future) {
95+
}
96+
};
97+
}
98+
}

0 commit comments

Comments
 (0)