Skip to content

Commit 6f51656

Browse files
pavel-ptashytscodex
andcommitted
Harden queued body read cancellation
Skip worker reads when cancellation closes a body stream before its queued task starts. This prevents closed streams from being touched and keeps cancelled uploads from consuming executor capacity. Add deterministic HTTP/1.1 and HTTP/2 queue tests, plus an HTTP/2 sibling stream test that proves a blocked upload does not stall another stream. Document executor-taking constructors as internal client wiring. Codex on behalf of Pavel Ptashyts Co-Authored-By: Codex <codex@openai.com>
1 parent eb08baf commit 6f51656

6 files changed

Lines changed: 279 additions & 0 deletions

File tree

‎client/src/main/java/org/asynchttpclient/DefaultAsyncHttpClient.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,11 @@ ChannelManager channelManager() {
134134
return channelManager;
135135
}
136136

137+
// Visible for testing
138+
@Nullable ExecutorService blockingBodyReadExecutor() {
139+
return blockingBodyReadExecutor;
140+
}
141+
137142
private static Timer newNettyTimer(AsyncHttpClientConfig config) {
138143
ThreadFactory threadFactory = config.getThreadFactory() != null ? config.getThreadFactory() : new DefaultThreadFactory(config.getThreadPoolName() + "-timer");
139144
HashedWheelTimer timer = new HashedWheelTimer(threadFactory, config.getHashedWheelTimerTickDuration(), TimeUnit.MILLISECONDS, config.getHashedWheelTimerSize());

‎client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,18 @@ public NettyRequestSender(AsyncHttpClientConfig config, ChannelManager channelMa
130130
this(config, channelManager, nettyTimer, clientState, Runnable::run);
131131
}
132132

133+
/**
134+
* Creates the request sender with the client-owned executor used for blocking request body reads.
135+
* This overload is public only for cross-package client wiring; applications should configure the
136+
* executor through {@link AsyncHttpClientConfig}.
137+
*
138+
* @param config client configuration
139+
* @param channelManager channel manager
140+
* @param nettyTimer request timer
141+
* @param clientState client lifecycle state
142+
* @param blockingBodyReadExecutor executor for blocking request body reads
143+
* @since 3.0.12
144+
*/
133145
public NettyRequestSender(AsyncHttpClientConfig config, ChannelManager channelManager, Timer nettyTimer,
134146
AsyncHttpClientState clientState, Executor blockingBodyReadExecutor) {
135147
this.config = config;

‎client/src/main/java/org/asynchttpclient/netty/request/body/NettyInputStreamBody.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,29 @@ public NettyInputStreamBody(InputStream inputStream, long contentLength) {
5555
this(inputStream, contentLength, Runnable::run, false);
5656
}
5757

58+
/**
59+
* Creates a request body whose stream reads are dispatched through the supplied executor.
60+
* This overload supports internal client wiring; applications should enable offloading through
61+
* {@link org.asynchttpclient.AsyncHttpClientConfig}.
62+
*
63+
* @param inputStream request body stream
64+
* @param blockingBodyReadExecutor executor for blocking reads
65+
* @since 3.0.12
66+
*/
5867
public NettyInputStreamBody(InputStream inputStream, Executor blockingBodyReadExecutor) {
5968
this(inputStream, -1L, blockingBodyReadExecutor);
6069
}
6170

71+
/**
72+
* Creates a request body whose stream reads are dispatched through the supplied executor.
73+
* This overload supports internal client wiring; applications should enable offloading through
74+
* {@link org.asynchttpclient.AsyncHttpClientConfig}.
75+
*
76+
* @param inputStream request body stream
77+
* @param contentLength request body length, or {@code -1} when unknown
78+
* @param blockingBodyReadExecutor executor for blocking reads
79+
* @since 3.0.12
80+
*/
6281
public NettyInputStreamBody(InputStream inputStream, long contentLength, Executor blockingBodyReadExecutor) {
6382
this(inputStream, contentLength, blockingBodyReadExecutor, true);
6483
}
@@ -263,6 +282,9 @@ private void submitReadAfterSuspend() throws IOException {
263282
}
264283

265284
private void readOffEventLoop() {
285+
if (closed) {
286+
return;
287+
}
266288
int read = -1;
267289
IOException thrown = null;
268290
try {
@@ -400,6 +422,9 @@ private void submitReadAfterSuspend() throws IOException {
400422
}
401423

402424
private void readOffEventLoop() {
425+
if (closed) {
426+
return;
427+
}
403428
int read = -1;
404429
IOException thrown = null;
405430
try {

‎client/src/test/java/org/asynchttpclient/BasicHttpTest.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import java.util.concurrent.CountDownLatch;
5656
import java.util.concurrent.ExecutionException;
5757
import java.util.concurrent.Future;
58+
import java.util.concurrent.ThreadPoolExecutor;
5859
import java.util.concurrent.TimeoutException;
5960
import java.util.concurrent.atomic.AtomicBoolean;
6061
import java.util.concurrent.atomic.AtomicInteger;
@@ -66,6 +67,9 @@
6667
import static io.netty.handler.codec.http.HttpHeaderNames.TRANSFER_ENCODING;
6768
import static java.nio.charset.StandardCharsets.UTF_8;
6869
import static java.util.concurrent.TimeUnit.SECONDS;
70+
import static org.asynchttpclient.OffloadedBodyReadTestSupport.BlockingInputStream;
71+
import static org.asynchttpclient.OffloadedBodyReadTestSupport.CloseProbeInputStream;
72+
import static org.asynchttpclient.OffloadedBodyReadTestSupport.awaitExecutorState;
6973
import static org.asynchttpclient.Dsl.config;
7074
import static org.asynchttpclient.Dsl.get;
7175
import static org.asynchttpclient.Dsl.head;
@@ -1130,6 +1134,48 @@ public int read(byte[] buffer, int offset, int length) {
11301134
});
11311135
}
11321136

1137+
@RepeatedIfExceptionsTest(repeats = 5)
1138+
public void cancelledQueuedInputStreamIsNotReadOverHttp1() throws Throwable {
1139+
withServer(server).run(server -> {
1140+
server.enqueueEcho();
1141+
server.enqueueEcho();
1142+
byte[] bodyBytes = "{}".getBytes(StandardCharsets.ISO_8859_1);
1143+
BlockingInputStream blockingStream = new BlockingInputStream(bodyBytes);
1144+
CloseProbeInputStream queuedStream = new CloseProbeInputStream();
1145+
1146+
try (DefaultAsyncHttpClient client = new DefaultAsyncHttpClient(config()
1147+
.setRequestBodyStreamReadOffloadEnabled(true)
1148+
.setRequestBodyStreamReadThreadsCount(1)
1149+
.setRequestBodyStreamReadQueueSize(1)
1150+
.build())) {
1151+
ThreadPoolExecutor executor = assertInstanceOf(ThreadPoolExecutor.class,
1152+
client.blockingBodyReadExecutor());
1153+
ListenableFuture<Response> blockingUpload = client.preparePost(getTargetUrl())
1154+
.setBody(new InputStreamBodyGenerator(blockingStream, bodyBytes.length))
1155+
.execute();
1156+
assertTrue(blockingStream.readStarted.await(5, SECONDS),
1157+
"first body read should occupy the worker");
1158+
1159+
ListenableFuture<Response> queuedUpload = client.preparePost(getTargetUrl())
1160+
.setBody(new InputStreamBodyGenerator(queuedStream, 1))
1161+
.execute();
1162+
awaitExecutorState(executor, 1, 1);
1163+
1164+
assertTrue(queuedUpload.cancel(true), "queued request should be cancelled");
1165+
assertTrue(queuedStream.closed.await(5, SECONDS),
1166+
"cancellation should close the queued stream");
1167+
1168+
blockingStream.releaseRead.countDown();
1169+
assertEquals(200, blockingUpload.get(TIMEOUT, SECONDS).getStatusCode());
1170+
awaitExecutorState(executor, 0, 0);
1171+
assertFalse(queuedStream.readAttempted.get(),
1172+
"a queued read must not start after cancellation closed its stream");
1173+
} finally {
1174+
blockingStream.releaseRead.countDown();
1175+
}
1176+
});
1177+
}
1178+
11331179
private static final class EventLoopProbeInputStream extends InputStream {
11341180
private final byte[] data;
11351181
private final AtomicBoolean readAttempted = new AtomicBoolean();
@@ -1186,4 +1232,5 @@ private void recordReadThread() {
11861232
}
11871233
}
11881234
}
1235+
11891236
}

‎client/src/test/java/org/asynchttpclient/Http2StreamingBodyFlowControlTest.java‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
import java.time.Duration;
6262
import java.util.concurrent.CountDownLatch;
6363
import java.util.concurrent.ExecutionException;
64+
import java.util.concurrent.ThreadPoolExecutor;
6465
import java.util.concurrent.TimeUnit;
6566
import java.util.concurrent.atomic.AtomicBoolean;
6667
import java.util.concurrent.atomic.AtomicLong;
@@ -71,6 +72,9 @@
7172
import static java.util.concurrent.TimeUnit.SECONDS;
7273
import static org.asynchttpclient.Dsl.asyncHttpClient;
7374
import static org.asynchttpclient.Dsl.config;
75+
import static org.asynchttpclient.OffloadedBodyReadTestSupport.BlockingInputStream;
76+
import static org.asynchttpclient.OffloadedBodyReadTestSupport.CloseProbeInputStream;
77+
import static org.asynchttpclient.OffloadedBodyReadTestSupport.awaitExecutorState;
7478
import static org.junit.jupiter.api.Assertions.assertEquals;
7579
import static org.junit.jupiter.api.Assertions.assertFalse;
7680
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
@@ -362,6 +366,74 @@ public void inputStreamBodyReadsOffEventLoopOverHttp2() throws Exception {
362366
assertFalse(is.readOnEventLoop.get(), "InputStream.read must not run on an event-loop thread");
363367
}
364368

369+
@Test
370+
public void blockedInputStreamDoesNotStallSiblingStreamOverHttp2() throws Exception {
371+
startServer(-1);
372+
byte[] payload = deterministicPayload(SMALL_SIZE);
373+
BlockingInputStream bodyStream = new BlockingInputStream(payload);
374+
375+
try (AsyncHttpClient client = http2Client()) {
376+
ListenableFuture<Response> blockedUpload = client.preparePost(httpsUrl("/blocked"))
377+
.setBody(new InputStreamBodyGenerator(bodyStream, payload.length))
378+
.execute();
379+
assertTrue(bodyStream.readStarted.await(5, SECONDS), "blocking body read should start");
380+
381+
Response sibling = client.prepareGet(httpsUrl("/sibling"))
382+
.execute()
383+
.get(5, SECONDS);
384+
assertEquals(200, sibling.getStatusCode(),
385+
"a sibling stream should complete while the upload read is blocked");
386+
387+
bodyStream.releaseRead.countDown();
388+
assertEchoed(blockedUpload.get(30, SECONDS), payload);
389+
} finally {
390+
bodyStream.releaseRead.countDown();
391+
}
392+
}
393+
394+
@Test
395+
public void cancelledQueuedInputStreamIsNotReadOverHttp2() throws Exception {
396+
startServer(-1);
397+
byte[] payload = deterministicPayload(SMALL_SIZE);
398+
BlockingInputStream blockingStream = new BlockingInputStream(payload);
399+
CloseProbeInputStream queuedStream = new CloseProbeInputStream();
400+
401+
DefaultAsyncHttpClientConfig clientConfig = config()
402+
.setUseInsecureTrustManager(true)
403+
.setHttp2Enabled(true)
404+
.setMaxConnectionsPerHost(1)
405+
.setRequestBodyStreamReadOffloadEnabled(true)
406+
.setRequestBodyStreamReadThreadsCount(1)
407+
.setRequestBodyStreamReadQueueSize(1)
408+
.setRequestTimeout(Duration.ofSeconds(30))
409+
.build();
410+
411+
try (DefaultAsyncHttpClient client = new DefaultAsyncHttpClient(clientConfig)) {
412+
ThreadPoolExecutor executor = assertInstanceOf(ThreadPoolExecutor.class,
413+
client.blockingBodyReadExecutor());
414+
ListenableFuture<Response> blockingUpload = client.preparePost(httpsUrl("/blocking"))
415+
.setBody(new InputStreamBodyGenerator(blockingStream, payload.length))
416+
.execute();
417+
assertTrue(blockingStream.readStarted.await(5, SECONDS), "first body read should occupy the worker");
418+
419+
ListenableFuture<Response> queuedUpload = client.preparePost(httpsUrl("/queued"))
420+
.setBody(new InputStreamBodyGenerator(queuedStream, 1))
421+
.execute();
422+
awaitExecutorState(executor, 1, 1);
423+
424+
assertTrue(queuedUpload.cancel(true), "queued request should be cancelled");
425+
assertTrue(queuedStream.closed.await(5, SECONDS), "cancellation should close the queued stream");
426+
427+
blockingStream.releaseRead.countDown();
428+
assertEchoed(blockingUpload.get(30, SECONDS), payload);
429+
awaitExecutorState(executor, 0, 0);
430+
assertFalse(queuedStream.readAttempted.get(),
431+
"a queued read must not start after cancellation closed its stream");
432+
} finally {
433+
blockingStream.releaseRead.countDown();
434+
}
435+
}
436+
365437
@Test
366438
public void inputStreamRuntimeFailureFailsRequestOverHttp2() throws Exception {
367439
startServer(-1);
@@ -587,6 +659,7 @@ private void recordReadThread() {
587659
}
588660
}
589661

662+
590663
// Lifecycle (finding #2 from the cold audit): when the stream is closed while the body pump is parked on
591664
// SUSPEND (a streaming/feedable body with no data yet), no in-flight write's failure would run cleanup —
592665
// so the writer must close the body source from the stream's closeFuture, or the source (and any file
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
/*
2+
* Copyright (c) 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;
17+
18+
import java.io.IOException;
19+
import java.io.InputStream;
20+
import java.util.concurrent.CountDownLatch;
21+
import java.util.concurrent.ThreadPoolExecutor;
22+
import java.util.concurrent.atomic.AtomicBoolean;
23+
import java.util.concurrent.atomic.AtomicInteger;
24+
25+
import static java.util.concurrent.TimeUnit.MILLISECONDS;
26+
import static java.util.concurrent.TimeUnit.SECONDS;
27+
import static org.junit.jupiter.api.Assertions.assertEquals;
28+
29+
final class OffloadedBodyReadTestSupport {
30+
31+
private OffloadedBodyReadTestSupport() {
32+
}
33+
34+
static void awaitExecutorState(ThreadPoolExecutor executor, int queued, int active)
35+
throws InterruptedException {
36+
long deadline = System.nanoTime() + SECONDS.toNanos(5);
37+
while ((executor.getQueue().size() != queued || executor.getActiveCount() != active)
38+
&& System.nanoTime() < deadline) {
39+
new CountDownLatch(1).await(10, MILLISECONDS);
40+
}
41+
assertEquals(queued, executor.getQueue().size(), "unexpected body-read executor queue size");
42+
assertEquals(active, executor.getActiveCount(), "unexpected body-read executor active count");
43+
}
44+
45+
static final class BlockingInputStream extends InputStream {
46+
private final byte[] data;
47+
private final AtomicBoolean firstRead = new AtomicBoolean(true);
48+
private final AtomicInteger position = new AtomicInteger();
49+
final CountDownLatch readStarted = new CountDownLatch(1);
50+
final CountDownLatch releaseRead = new CountDownLatch(1);
51+
52+
BlockingInputStream(byte[] data) {
53+
this.data = data;
54+
}
55+
56+
@Override
57+
public int read() throws IOException {
58+
byte[] single = new byte[1];
59+
int read = read(single, 0, 1);
60+
return read < 0 ? -1 : single[0] & 0xFF;
61+
}
62+
63+
@Override
64+
public int read(byte[] buffer, int offset, int length) throws IOException {
65+
awaitFirstRead();
66+
int current = position.get();
67+
if (current == data.length) {
68+
return -1;
69+
}
70+
int read = Math.min(length, data.length - current);
71+
System.arraycopy(data, current, buffer, offset, read);
72+
position.addAndGet(read);
73+
return read;
74+
}
75+
76+
@Override
77+
public void close() {
78+
releaseRead.countDown();
79+
}
80+
81+
private void awaitFirstRead() throws IOException {
82+
if (firstRead.compareAndSet(true, false)) {
83+
readStarted.countDown();
84+
try {
85+
if (!releaseRead.await(10, SECONDS)) {
86+
throw new IOException("timed out waiting to release blocking request body read");
87+
}
88+
} catch (InterruptedException e) {
89+
Thread.currentThread().interrupt();
90+
throw new IOException("blocking request body read interrupted", e);
91+
}
92+
}
93+
}
94+
}
95+
96+
static final class CloseProbeInputStream extends InputStream {
97+
final AtomicBoolean readAttempted = new AtomicBoolean();
98+
final CountDownLatch closed = new CountDownLatch(1);
99+
100+
@Override
101+
public int read() {
102+
readAttempted.set(true);
103+
return -1;
104+
}
105+
106+
@Override
107+
public int read(byte[] buffer, int offset, int length) {
108+
readAttempted.set(true);
109+
return -1;
110+
}
111+
112+
@Override
113+
public void close() {
114+
closed.countDown();
115+
}
116+
}
117+
}

0 commit comments

Comments
 (0)