-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add response body flow control #2318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| /* | ||
| * Copyright (c) 2026 AsyncHttpClient Project. All rights reserved. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.asynchttpclient; | ||
|
|
||
| /** | ||
| * Controls transport reads for a response body. | ||
| * <p> | ||
| * The control is thread-safe and remains valid until its response completes. Calls made after completion have no | ||
| * effect. | ||
| * | ||
| * @since 3.0.14 | ||
| */ | ||
| public interface ResponseBodyControl { | ||
|
|
||
| /** | ||
| * Stops requesting additional response bytes from the transport. Body parts that were already read may still be | ||
| * delivered to the {@link AsyncHandler}. | ||
| * <p> | ||
| * While reads are suspended, the read timeout is paused but the request timeout remains active. If the request | ||
| * timeout is disabled, failing to resume or cancel the response can retain its transport resources indefinitely. | ||
| * <p> | ||
| * For HTTP/2, AHC continues returning connection-level flow-control credit so a suspended stream cannot block | ||
| * sibling streams. The per-stream window still applies, so roughly | ||
| * {@link AsyncHttpClientConfig#getHttp2InitialWindowSize()} bytes can be queued for each suspended stream. Aggregate | ||
| * buffering can therefore scale with the number of concurrent suspended streams; applications can bound it with | ||
| * {@link AsyncHttpClientConfig#getHttp2InitialWindowSize()} and | ||
| * {@link AsyncHttpClientConfig#getHttp2MaxConcurrentStreams()}. | ||
| */ | ||
| void suspend(); | ||
|
|
||
| /** | ||
| * Resumes requesting response bytes after a call to {@link #suspend()}. | ||
| */ | ||
| void resume(); | ||
|
|
||
| /** | ||
| * Stops processing the response body. As with {@link AsyncHandler.State#ABORT}, the handler is completed normally. | ||
| * Returning {@code ABORT} is the preferred way to stop from within an {@link AsyncHandler} callback; this method is | ||
| * intended for cancellation after the callback has returned, including from another thread. | ||
| */ | ||
| void cancel(); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -91,6 +91,7 @@ public void channelInactive(ChannelHandlerContext ctx) throws Exception { | |
| } | ||
|
|
||
| Channel channel = ctx.channel(); | ||
| NettyResponseBodyControl.discardForChannelClose(channel); | ||
| channelManager.removeAll(channel); | ||
|
|
||
| Object attribute = Channels.getAttribute(channel); | ||
|
|
@@ -122,6 +123,7 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable e) { | |
| } | ||
|
|
||
| Channel channel = ctx.channel(); | ||
| NettyResponseBodyControl.discardForChannelClose(channel); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This runs before the two branches below that keep the exchange alive on purpose, the IOException filter one and |
||
| NettyResponseFuture<?> future = null; | ||
|
|
||
| logger.debug("Unexpected I/O exception on channel {}", channel, cause); | ||
|
|
@@ -179,7 +181,9 @@ public void channelActive(ChannelHandlerContext ctx) { | |
|
|
||
| @Override | ||
| public void channelReadComplete(ChannelHandlerContext ctx) { | ||
| readIfNeeded(ctx); | ||
| if (!NettyResponseBodyControl.isSuspended(ctx.channel())) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: this does an attribute lookup on every read complete for every channel, including each H2 stream child, and in the normal case |
||
| readIfNeeded(ctx); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -196,6 +200,7 @@ private static void readIfNeeded(ChannelHandlerContext ctx) { | |
| } | ||
|
|
||
| void finishUpdate(NettyResponseFuture<?> future, Channel channel, boolean close) { | ||
| NettyResponseBodyControl.complete(channel); | ||
| future.cancelTimeouts(); | ||
|
|
||
| if (close) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -180,14 +180,26 @@ private void handleHttp2HeadersFrame(Http2HeadersFrame headersFrame, Channel cha | |
| if (!abort) { | ||
| abort = handler.onHeadersReceived(responseHeaders) == State.ABORT; | ||
| } | ||
| if (!abort) { | ||
| NettyResponseBodyControl control = NettyResponseBodyControl.create( | ||
| channel, future::touch, () -> finishUpdate(future, channel, false)); | ||
| abort = handler.onResponseBodyStart(control) == State.ABORT; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When the HEADERS frame ends the stream we still create a control and call this, and |
||
| if (abort) { | ||
| NettyResponseBodyControl.complete(channel); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| } | ||
| if (abort) { | ||
| finishUpdate(future, channel, false); | ||
| // cancel() may have completed the future inline from onResponseBodyStart. | ||
| if (!future.isDone()) { | ||
| finishUpdate(future, channel, false); | ||
| } | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| // If headers frame also ends the stream (no body), finish the response | ||
| if (headersFrame.isEndStream()) { | ||
| // unless cancel() already completed it inline from onResponseBodyStart. | ||
| if (headersFrame.isEndStream() && !future.isDone()) { | ||
| finishUpdate(future, channel, false); | ||
| } | ||
| } | ||
|
|
@@ -205,6 +217,10 @@ private void handleHttp2DataFrame(Http2DataFrame dataFrame, Channel channel, | |
| if (data.isReadable() || last) { | ||
| HttpResponseBodyPart bodyPart = config.getResponseBodyPartFactory().newResponseBodyPart(data, last); | ||
| boolean abort = handler.onBodyPartReceived(bodyPart) == State.ABORT; | ||
| // cancel() may have completed the future inline from the handler callback. | ||
| if (future.isDone()) { | ||
| return; | ||
| } | ||
| if (abort || last) { | ||
| finishUpdate(future, channel, false); | ||
| } | ||
|
|
@@ -224,6 +240,10 @@ private void handleHttp2TrailingHeadersFrame(Http2HeadersFrame headersFrame, Cha | |
| boolean abort = false; | ||
| if (!trailingHeaders.isEmpty()) { | ||
| abort = handler.onTrailingHeadersReceived(trailingHeaders) == State.ABORT; | ||
| // cancel() may have completed the future inline from the handler callback. | ||
| if (future.isDone()) { | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| if (abort || headersFrame.isEndStream()) { | ||
|
|
@@ -285,6 +305,7 @@ private void handleHttp2ResetFrame(Http2ResetFrame resetFrame, Channel channel, | |
| */ | ||
| @Override | ||
| void finishUpdate(NettyResponseFuture<?> future, Channel streamChannel, boolean close) { | ||
| NettyResponseBodyControl.complete(streamChannel); | ||
| future.cancelTimeouts(); | ||
|
|
||
| // Stream channels are single-use in HTTP/2 — close the stream | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,17 @@ private static boolean abortAfterHandlingHeaders(AsyncHandler<?> handler, HttpHe | |
| return !responseHeaders.isEmpty() && handler.onHeadersReceived(responseHeaders) == State.ABORT; | ||
| } | ||
|
|
||
| private boolean abortAfterStartingResponseBody(Channel channel, NettyResponseFuture<?> future, | ||
| AsyncHandler<?> handler) throws Exception { | ||
| NettyResponseBodyControl control = NettyResponseBodyControl.create( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| channel, future::touch, () -> finishUpdate(future, channel, true)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| boolean abort = handler.onResponseBodyStart(control) == State.ABORT; | ||
| if (abort) { | ||
| NettyResponseBodyControl.complete(channel); | ||
| } | ||
| return abort; | ||
| } | ||
|
|
||
| private void handleHttpResponse(final HttpResponse response, final Channel channel, final NettyResponseFuture<?> future, AsyncHandler<?> handler) throws Exception { | ||
| HttpRequest httpRequest = future.getNettyRequest().getHttpRequest(); | ||
| if (logger.isDebugEnabled()) { | ||
|
|
@@ -68,8 +79,11 @@ private void handleHttpResponse(final HttpResponse response, final Channel chann | |
| HttpHeaders responseHeaders = response.headers(); | ||
|
|
||
| if (!interceptors.exitAfterIntercept(channel, future, handler, response, status, responseHeaders)) { | ||
| boolean abort = abortAfterHandlingStatus(handler, httpRequest.method(), status) || abortAfterHandlingHeaders(handler, responseHeaders); | ||
| if (abort) { | ||
| boolean abort = abortAfterHandlingStatus(handler, httpRequest.method(), status) | ||
| || abortAfterHandlingHeaders(handler, responseHeaders) | ||
| || abortAfterStartingResponseBody(channel, future, handler); | ||
| // cancel() may have completed the future inline from onResponseBodyStart. | ||
| if (abort && !future.isDone()) { | ||
| finishUpdate(future, channel, true); | ||
| } | ||
| } | ||
|
|
@@ -92,6 +106,10 @@ private void handleChunk(HttpContent chunk, final Channel channel, final NettyRe | |
| if (!abort && (buf.isReadable() || last)) { | ||
| HttpResponseBodyPart bodyPart = config.getResponseBodyPartFactory().newResponseBodyPart(buf, last); | ||
| abort = handler.onBodyPartReceived(bodyPart) == State.ABORT; | ||
| // cancel() may have completed the future inline from the handler callback. | ||
| if (future.isDone()) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The same guard is missing after |
||
| return; | ||
| } | ||
| } | ||
|
|
||
| if (abort || last) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This turns the connection level bound off for every H2 connection, not only the ones that use the new control. With the shipped defaults (
http2InitialWindowSize16 MiB,http2MaxConcurrentStreams-1) nothing aggregate is left, so the ceiling becomes the per stream window times whatever concurrency the server picks. Before this a slow consumer was capped at 64 KiB per connection.Can we auto refill only while a control on that connection is actually suspended, or keep an aggregate byte budget? I would rather not change the default for users who never touch
ResponseBodyControl. The README points athttp2MaxConcurrentStreamsas the mitigation, but that is off by default too.