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

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, "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:

• 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.
• 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.
• 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.
• 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:

• ResponseBodyControlTest, Http2ResponseBodyControlTest and NettyResponseBodyControlTest on JDK 11: 31 tests passed.
• ./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.
• The same 31 tests, compiled on JDK 11 and run on JDK 17, 21 and 25 (Maven Surefire's -Djvm): 0 failures on each.
• 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 fix(netty): stop inflating a suspended HTTP/1.1 response #2362.