feat: add ResponseBodyControl.execute to decide in sequence with body delivery by mkurz · Pull Request #2359 · AsyncHttpClient/async-http-client

mkurz · 2026-10-07T08:09:05Z

Summary

Add ResponseBodyControl.execute(Runnable), which runs a task on the event loop that delivers the response body, in sequence with the callbacks AHC makes there, such as onBodyPartReceived. Calls to suspend(), resume() and cancel() from the task are applied before they return, not queued.
This lets a consumer that buffers body parts on another thread decide whether to resume based on everything delivered so far, instead of on a decision that a part delivered in the meantime made stale.
NettyResponseBodyControl already applied its own control calls this way; that method is now public. The default implementation runs the task on the calling thread.

Problem

A typical streaming consumer buffers body parts from onBodyPartReceived and hands them to a reader on another thread. When that reader has emptied the buffer and still wants more, it calls resume(). Called from that thread, resume() only takes effect later, on the event loop. In between, the event loop can still deliver parts from the current socket read, and those parts may already meet the reader's demand. The queued resume() then lets one more socket read through that nobody asked for.

With a highly compressible body, that one extra read can inflate to many megabytes beyond demand. For a gzipped stream with early demand from another thread, Play WS measured about 35 MB buffered beyond demand, against about 1.9 MB when the decision is re-checked on the event loop.

AHC cannot fix this by re-checking the decision itself, because only the consumer knows which state its decision depends on. Play WS currently works around it by reaching the event loop through Netty's internal ThreadExecutorMap, and then making its resume decision there.

Change

default void execute(Runnable task) {
    Objects.requireNonNull(task, "task");
    task.run();
}

On the event loop, for example from within a body callback, the task runs before execute returns.
From another thread, it is queued to the event loop and runs there after the work already pending on it. The event loop can deliver further body parts before the task runs; the task sees all of them.
Either way, suspend(), resume() and cancel() called from the task are applied before they return, not queued. As suspend() already says, parts that were already read can still be delivered after it. resume() can deliver parts, and resume() and cancel() can complete the response, before they return: on HTTP/2, resume() reads the frames queued on the stream, and cancel() runs onCompleted.
onThrowable runs once the control is inactive, so control calls from a task passed from there have no effect. For most failures, such as an I/O error, a closed connection or a reset stream, it runs on the event loop, and the task runs before execute returns. A cancel or abort from another thread calls it on that thread, and a timeout on the timer thread unless useEventLoopTimeouts is enabled. From there, the task is queued and can run while that callback is still running.
The task must not throw. An exception from a task that runs before execute returns is thrown from execute, so a callback that lets it propagate fails the response. An exception from a queued task is logged by the event loop and does not fail the response. If that task was to resume reads, the response stays suspended until it is resumed or cancelled, or the request timeout fires; with the request timeout disabled, indefinitely, since the read timeout is paused while reads are suspended. The Javadoc says so.
The task must not block. Unlike calls to the other methods, it also runs after the response has completed (the interface Javadoc now says so). If the event loop no longer accepts tasks, it does not run.
The default implementation runs the task on the calling thread. It suits implementations that apply control calls synchronously on the calling thread, and does not order the task with callbacks on other threads. An implementation that delegates to another control must delegate execute too.

NettyResponseBodyControl.execute is the existing private method made public, with a null check. It works for HTTP/1.1 and for HTTP/2 stream channels.

Alternatives considered

resumeIf(BooleanSupplier), a resume whose condition AHC evaluates on the event loop. It needs subtle ordering rules: a true condition counts as a resume at that moment and overrides an earlier suspend. A consumer whose own drain loop may run on either thread then has to fit its state machine to those rules, and we found that hard to get right. With execute, such a consumer can write control.execute(() -> { if (cond) control.resume(); }).
Demand counting in AHC (request(n)). HTTP/1.1 decodes one socket read into several parts regardless of demand, so AHC would need its own buffer. That's a much larger change.

execute only exposes what the control already does internally. Callbacks already run on the event loop under the same rule not to block.

Compatibility

A new default method on ResponseBodyControl. Revapi passed. Existing implementations keep compiling and behave as before, with two exceptions:
- an implementation that declares a non-public execute(Runnable) no longer compiles;
- one that already has a public void execute(Runnable) now implements this method, with whatever semantics it has.
No behavior change for existing callers.

AI disclosure

Claude Code on behalf of Matthias Kurz. The commits include Co-Authored-By per AGENTS.md.

Test plan

New tests:

ResponseBodyControlTest:
- execute runs inline in a callback, runs on the event loop when called from another thread, and runs after control calls queued before it;
- a consumer thread that decides inside execute sees the part delivered in the meantime and does not resume;
- execute after the response has completed still runs the task on the event loop;
- a queued task that throws does not fail the response: it stays suspended, a later task resumes it, it completes, and its connection is reused;
- a queued task that throws after the response has completed leaves the pooled connection open and reused;
- a task that throws on the event loop throws from execute, and the response completes when the caller catches it;
- the default implementation runs the task on the calling thread, and rejects a null task with the message task.
Http2ResponseBodyControlTest:
- a task from another thread runs on the thread that delivers the stream's body parts, and resumes the suspended stream;
- a queued task that throws does not fail the stream, and a later task resumes it.
NettyResponseBodyControlExecuteTest: once the event loop has shut down, execute neither runs the task nor throws.

Broken variants, at the first revision, each run against these tests and the existing control tests (24 tests in all):

The task always runs on the calling thread: 9 failures, including "a task from another thread must run on the channel event loop".
The task is always queued: 3 failures, including "a task from a callback must run before execute returns".
A rejected task throws: the shutdown test fails.
No task after completion: the completion test times out (plus failures of existing tests, since the control's own calls use the same method).

Also at the first revision, deciding on the other thread without execute failed "the task must see the part buffered after it was queued".

Broken variants for the review changes, each run against the control tests:

An exception from a queued task fails the response: 2 failures, the HTTP/1.1 and HTTP/2 tests for a queued task that throws.
An exception from a task that runs before execute returns is swallowed: 1 failure, the inline test.
Tasks from another thread run on a pool thread, while the control's own calls stay on the event loop: 4 failures, including the HTTP/2 thread check. With the previous revision's assertNotSame, the HTTP/2 test passed.

Runs:

Each new commit compiles with its tests.
./mvnw -B -ntp -Dgpg.skip=true clean verify on JDK 11: 1,802 tests, 0 failures, 0 errors, 11 skipped; Revapi passed. No test-skipping flags were used.
The control tests (*ResponseBodyControl*Test and NettyResponseFutureTest), compiled on JDK 11 and run on JDK 17, 21 and 25 (Maven Surefire's -Djvm): 40 tests each, 0 failures.
Play WS, with its resume check moved from ThreadExecutorMap to execute, against a local build of the first revision of this branch alone (Scala 2.13):
- all Play WS tests pass, except a new cookie test that needs the separate raw-Cookie fix;
- its gzip streaming test buffers 1,875,916 bytes beyond demand, as with the ThreadExecutorMap workaround;
- with Play WS resuming directly instead of through execute, the same test buffers 35,589,292 bytes and fails.
The review changes are Javadoc, the null check in the default implementation, and tests.
All four open AHC changes merged together (raw Cookie headers on retries, ResponseBodyControl.execute, suspend/resume order, demand-bounded decompression): they merge without conflicts, together and in every pair, and ./mvnw -B -ntp -Dgpg.skip=true clean install on JDK 11 passes with 1,872 tests, 0 failures, 0 errors, 11 skipped; Revapi passed.
Play WS against that merged build, Scala 2.13 and 3.3, JDK 17: all tests pass. The shaded AHC jar contains this revision's default execute.

A consumer that buffers body parts on another thread decides there whether to resume reads, but resume() from that thread only takes effect later on the event loop. In between, the event loop can deliver another part that already meets the consumer's demand, and the resume then lets one more read through. With a highly compressible body, that read can inflate to many megabytes beyond demand. Only the caller knows which state its decision depends on, so AHC cannot re-check it. Add execute(Runnable), which runs a task on the event loop that delivers the response body, in sequence with the callbacks made there: before returning when called on that event loop, and queued to it otherwise. suspend(), resume() and cancel() called from the task take effect immediately, so a consumer can decide there on everything delivered so far. onThrowable can run on other threads, after a cancel or a timeout, so a task is not ordered with it. Unlike the other methods, the task also runs after completion; it does not run once the event loop rejects tasks. NettyResponseBodyControl already applied its own control calls this way; that method is now public. The default implementation runs the task on the calling thread, for implementations that apply control calls synchronously. Play WS needs this: it currently reaches the event loop through Netty's internal ThreadExecutorMap to avoid the extra read. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The Javadoc said a task passed to execute from onThrowable is queued, as if that callback only ran on a cancelling thread or the timer thread. Most failures, such as an I/O error, a closed connection or a reset stream, call it on the event loop, so the task runs before execute returns. On every path the control is inactive by then, so control calls from the task have no effect. Timeouts run on the event loop only with useEventLoopTimeouts, which is off by default. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The Javadoc said suspend, resume and cancel called from a task take effect immediately. They are applied before they return, but suspend does not hold back parts that were already read, as its own Javadoc says. On HTTP/2, resume reads the frames queued on the stream before it returns, so onBodyPartReceived and onCompleted can run inside it. cancel completes the response before it returns too. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A task that execute queued from another thread runs as a plain event loop task: Netty logs an exception from it, and the response carries on. A task that runs before execute returns throws to its caller, and from a callback that fails the response. Neither was documented. Say that the task must not throw, and what happens if it does. A queued task meant to resume reads that throws leaves the response suspended until it is resumed or cancelled, or the request timeout fires. Without a request timeout that can be indefinitely, since the read timeout is paused while reads are suspended. The tests pin both cases, and that a task throwing after completion leaves the pooled connection alone. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A control written against 3.0.14 that forwards suspend, resume and cancel to another control gets the default execute, which runs the task on the calling thread and so loses the ordering execute is for. Say that a delegating control must delegate execute too. The default also rejects a null task with a message now, as NettyResponseBodyControl does. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

assertNotSame against the test thread passed for a task run on any other thread. Had execute run it on a pool thread, the resume() inside would still have been queued to the event loop, and the request would still have completed. Check that the task ran on the thread that delivered the body parts, the stream's event loop. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

mkurz · 2026-10-23T16:44:25Z

Thanks for the review. I've pushed five commits on top and updated the description. Replies are inline.

## Summary - A `ResponseBodyControl.suspend()` queued from another thread no longer takes effect after a `resume()` that the event loop made in the meantime, which could leave a response suspended with nothing left to resume it. - A `resume()` is still always applied, so it is never lost to a `suspend()` that a callback makes after it was queued. - Document when control calls take effect. No API change. ## Problem `NettyResponseBodyControl` applies a call made on the event loop before it returns, and queues a call made on any other thread to the event loop. Queued calls take effect in the order they were queued, without regard to calls the event loop made inline in the meantime. The stall: 1. A consumer thread calls `suspend()`, which is queued. 2. Before it runs, a body callback on the event loop calls `resume()`, which takes effect immediately. 3. The queued `suspend()` then runs and suspends reads again. Nothing calls `resume()` after that, so the response stalls until the request timeout. The callback's `resume()` was the latest decision, but the older `suspend()` overrode it. This pattern occurs in reactive consumers that suspend from whichever thread finds the buffer full. Play WS hit a variant of it, and avoids it today by suspending only on the event loop. ## Change - `suspend()` records a suspend request, and `resume()` clears it. On the event loop, `suspend()` suspends at once. From another thread, it queues the suspension. - A queued suspension is skipped if, when it would take effect, the most recent call to `suspend()` or `resume()` was a resume. - `resume()` is always applied, and calls made on the event loop take effect before they return, as before. This is not "the latest call wins". Only suspensions can be skipped. Skipping a resume instead would reintroduce a lost-wakeup stall: a consumer takes the last buffered part and calls `resume()`, a callback on an older view calls `suspend()` before that resume is applied, and the resume must still win. A test pins this case. Calls made concurrently on different threads are not ordered beyond these rules. For example, with suspend, resume and suspend queued from other threads, the first suspension still takes effect briefly, because the most recent call is a suspend when it runs; the queued resume and the second suspension then follow. The `ResponseBodyControl` Javadoc now states when calls take effect: - a call on the event loop takes effect before it returns; - calls from other threads take effect later on the loop, possibly after further callbacks; - a `resume()` always takes effect; - a `suspend()` from another thread is skipped if the most recent call was a resume; - concurrent calls on different threads are not ordered otherwise. ## Compatibility No public API change. Behavior changes only for a `suspend()` from another thread whose queued suspension is overtaken by a `resume()`: it no longer takes effect. All other call orders behave as before. ## Related The separate pull request [#2359](#2359), "feat: add ResponseBodyControl.execute to decide in sequence with body delivery", lets a consumer make its suspend/resume decision on the event loop in the first place. The two changes are independent and merge cleanly with each other. ## AI disclosure Claude Code on behalf of Matthias Kurz. The commits include `Co-Authored-By` per `AGENTS.md`. ## Test plan New tests, for HTTP/1.1, for HTTP/2, and as unit tests in `NettyResponseBodyControlTest` (with Netty auto-read on and off): - an earlier `suspend()` from another thread does not undo a later `resume()`; - a `resume()` from another thread is not lost to a later `suspend()`. `NettyResponseBodyControlTest` also covers: - sequences of calls queued from another thread: suspend, resume, suspend ends suspended, and resume, suspend, resume ends resumed, with the exact number of suspensions started and ended; - a `suspend()` after a skipped one still takes effect; - a `suspend()` on the event loop takes effect even if a `resume()` from another thread clears its request between recording and acting on it. The channel's event loop is a proxy whose `inEventLoop()` runs that resume at exactly that moment. The unit tests also check that the suspension start/end hooks stay balanced and that exactly one read is requested per resume. Calls from another thread go through a single-thread executor owned by each test, not the common pool. Broken variants, each run against the 31 control tests: - [x] A queued suspension always applied, as on `main`: 7 failures and 1 error (HTTP/2, by timeout). These are the tests that an earlier `suspend()` must not undo a later `resume()`, the sequence test, and the test of a suspension after a skipped one. - [x] The latest call always wins, so a queued `resume()` is dropped for a later `suspend()`: 5 failures and 1 error (HTTP/2, by timeout). These are the tests that a `resume()` must not be lost, and the sequence test. - [x] A skipped suspension that still runs the suspension start hook: 4 failures, all in `NettyResponseBodyControlTest`, through the counts of started suspensions. The integration tests don't catch this variant. - [x] A `suspend()` on the event loop that still goes through the queued-suspend check, as in the first revision: 1 failure, the race test. Runs: - [x] `ResponseBodyControlTest`, `Http2ResponseBodyControlTest` and `NettyResponseBodyControlTest` on JDK 11: 31 tests passed. - [x] `./mvnw -B -ntp -Dgpg.skip=true clean verify` on JDK 11: 1,804 tests, 0 failures, 0 errors, 11 skipped; Revapi passed. No test-skipping flags were used. - [x] The same 31 tests, compiled on JDK 11 and run on JDK 17, 21 and 25 (Maven Surefire's `-Djvm`): 0 failures on each. - [x] All four open AHC changes merged together (raw `Cookie` headers on retries, `ResponseBodyControl.execute`, suspend/resume order, demand-bounded decompression): they merge without conflicts, together and in every pair, and `./mvnw -B -ntp -Dgpg.skip=true clean install` on JDK 11 passes with 1,861 tests, 0 failures, 0 errors, 11 skipped; Revapi passed. Rerun with the review fixes of this pull request and #2362. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Brings in AsyncHttpClient#2359 and AsyncHttpClient#2361, which also change NettyResponseBodyControl. The only conflict was the field block: keep the volatile int active flag from this branch alongside the new suspendRequested flag. Claude Code on behalf of Pavel Ptashyts Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

## Summary - Upgrades Netty from 4.2.19.Final to 4.2.20.Final. Its [security fixes](https://github.com/netty/netty/releases/tag/netty-4.2.20.Final) include several in modules AHC uses: three out-of-bounds reads in `netty-buffer`, unbounded memory usage in `netty-codec-http2`, insufficient control character neutralization in `netty-codec-http`, insufficient entropy in `netty-resolver-dns`, and an mTLS bypass via session resumption in `netty-handler`. - On 4.2.20, two of AHC's tests fail, both because of Netty changes. This PR adapts AHC to them. ## Change - **Upgrade Netty to 4.2.20.Final, and check the second gzip magic byte:** `JdkZlibDecoder` now rejects a gzip header with a wrong second magic byte (netty/netty#17972). The gzip inflate loop from #2357 follows `JdkZlibDecoder`, and `Http1ContentDecompressorTest` compares the two, so it now checks that byte too. Both bytes are read unsigned, since `0x8b` is negative as a signed byte. - **Keep zstd working at a non-positive threshold:** since netty/netty#17896, the `ZstdDecoder` that `HttpContentDecompressor` creates sizes its output buffers by `io.netty.compression.defaultMaxForwardBytes`. At 0 it hands on an empty body without an error; below 0 it fails with `IllegalArgumentException: initialCapacity : -1`. AHC leaves zstd to that decoder while the property is not positive (#2362), so it now replaces it with a `ZstdDecoder` whose buffers hold `MAX_PART_SIZE`. The size limit still applies, and the body is still decompressed eagerly there, as before. At the default value of the property nothing changes. netty/netty#18289 fixes the decoder in Netty; this workaround is harmless with that fix and can go once AHC requires a Netty version that has it. ## Notes - Netty 4.2.0 to 4.2.4 have no `ZstdDecoder(int)` constructor. Overriding AHC's Netty with one of those and also setting the property to 0 or below would fail zstd responses with a `NoSuchMethodError`. ## AI disclosure Claude Code on behalf of Matthias Kurz. Both commits include `Co-Authored-By` per `AGENTS.md`. ## Test plan - [x] `main` at `5542c9fc4` with `-Dnetty.version=4.2.20.Final`: 1,857 tests, 2 failures: - `Http1ContentDecompressorTest.decodesLikeNettyForEveryBodyAndSplit`, case "gzip second magic byte ignored": Netty now fails that body, AHC did not. - `Http1ContentDecompressorForwardThresholdTest`: "the zstd body differs". - [x] `Http1ContentDecompressorForwardThresholdTest` now runs at thresholds 0 and -1, each in a fresh JVM, and also checks the size limit for zstd and that every exchange releases its input and buffers. On 4.2.20 without the zstd commit it fails at both ("the zstd body differs", "the zstd body did not end"). With it, it passes on 4.2.19 and 4.2.20. - [x] `./mvnw -B -ntp -Dgpg.skip=true clean verify` on JDK 11, on `main` `3e95ec8c2`: 1,905 tests, 0 failures, 0 errors, 11 skipped; Revapi passed. No test-skipping flags were used. - [x] `Http1ContentDecompressorForwardThresholdTest` and `Http1ContentDecompressorTest` compiled on JDK 11 and run on JDK 17, 21 and 25 (Maven Surefire's `-Djvm`): 36 tests each, 0 failures. - [x] This branch merged with #2369: `clean verify` on JDK 11, 1,914 tests, 0 failures, 0 errors, 11 skipped; Revapi passed (run before #2359, #2360 and #2361 were merged, with those three merged in as well). --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

mkurz mentioned this pull request Oct 7, 2026

fix(netty): never let a queued suspend override a later resume #2361

Merged

8 tasks done

mkurz force-pushed the feature/response-body-control-execute branch from bcee2e0 to 0fd1488 Compare October 7, 2026 08:45

mkurz mentioned this pull request Oct 7, 2026

Upgrade to AHC 3.0.15 and decide streamed-body resumes through ResponseBodyControl.execute playframework/play-ws#1487

Draft

18 of 22 tasks

hyperxpro requested changes Oct 22, 2026

View reviewed changes

mkurz and others added 5 commits October 23, 2026 16:33

mkurz requested a review from hyperxpro October 23, 2026 16:45

mkurz mentioned this pull request Nov 4, 2026

fix(netty): stop inflating a suspended HTTP/1.1 response #2362

Merged

14 tasks done

hyperxpro approved these changes Nov 5, 2026

View reviewed changes

hyperxpro merged commit 7aca1b9 into AsyncHttpClient:main Nov 5, 2026
13 checks passed

mkurz deleted the feature/response-body-control-execute branch November 5, 2026 20:26

mkurz mentioned this pull request Nov 5, 2026

Upgrade Netty to 4.2.20.Final #2375

Merged

5 tasks done

Navigation Menu

Repository navigation

feat: add ResponseBodyControl.execute to decide in sequence with body delivery - #2359

feat: add ResponseBodyControl.execute to decide in sequence with body delivery#2359
hyperxpro merged 6 commits into
AsyncHttpClient:mainfrom
mkurz:feature/response-body-control-execute

mkurz commented Oct 7, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkurz commented Oct 23, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mkurz commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Change

Alternatives considered

Compatibility

AI disclosure

Test plan

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkurz commented Oct 23, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mkurz commented Oct 7, 2026 •

edited

Loading