IoUring: Stop generic FileRegion drain loop when transferred() reaches count() by LuciferYang · Pull Request #16826 · netty/netty

LuciferYang · 2026-05-19T05:25:19Z

Motivation

The generic FileRegion fallback added in #16571 drains the region only on buf.writableBytes() > 0:

while (buf.writableBytes() > 0) {
    long t = region.transferTo(ch, region.transferred());
    if (t <= 0) {
        break;
    }
}

epoll's equivalent at AbstractEpollStreamChannel#writeFileRegion guards the call with:

if (region.transferred() >= region.count()) {
    in.remove();
    return 0;
}

so epoll never invokes transferTo() past transferred() == count(). The io_uring fallback can, which violates the FileRegion contract for implementations that lazily emit per-chunk framing — the extra call builds an empty next chunk and ships its framing into the buffer, breaking the receiver. See #16825 for the full diagnosis.

Modification

Bound the inner drain loop with region.transferred() < region.count() in addition to buf.writableBytes() > 0, mirroring epoll's short-circuit.

Add IoUringSocketFileRegionTest#testFileRegionDrainStopsAtCompletion plus an OvershootDetectingFileRegion fixture that records every transferTo() invocation made past transferred() == count(). The fixture writes one byte and advances transferred to count in one shot, exercising the buggy path (15 overshoots on default chunk size) and the fixed path (0 overshoots) deterministically.

A few test-design notes for review:

Skips on non-io_uring client permutations — the drain loop is on the sending side.
Skips when io.netty.iouring.fileRegionChunkSize is set to 1, since that leaves no spare capacity for the overshoot to land in.
Asserts on the writer-side counter synchronously after writeAndFlush(...).sync(); no time-based wait.
Routes contract violations through IOException rather than AssertionError so they surface on the write future's cause through the transport's catch (Exception) instead of wedging the EventLoop.
@Timeout(value = 30, unit = SECONDS) as a safety net for any future regression that wedges sync().

Result

io_uring's generic FileRegion path stops over-calling transferTo(). Custom FileRegion implementations that lazily emit per-chunk framing work under io_uring with the same semantics they already have under NIO and epoll.

…s count() Motivation: The generic FileRegion fallback added in netty#16571 drains the region by looping on `buf.writableBytes() > 0` only. epoll's writeFileRegion() guards the same call with `if (region.transferred() >= region.count()) { in.remove(); return 0; }` -- the FileRegion contract permits implementations to assume transferTo() is not invoked past transferred() == count(). Calling transferTo() past completion can trigger side effects in custom FileRegion implementations that lazily emit per-chunk framing -- for example an encryption, compression, or framing layer that flushes a completed inner chunk on each call -- producing a phantom chunk header that lands on the wire and corrupts the receiver's framing. Modification: Tighten the io_uring drain-loop condition with `region.transferred() < region.count()` in addition to `buf.writableBytes() > 0`, mirroring epoll's short-circuit. Add IoUringSocketFileRegionTest#testFileRegionDrainStopsAtCompletion plus an OvershootDetectingFileRegion fixture that records every transferTo() call made past transferred() == count(). The test fails on the prior implementation and passes with this change. Result: io_uring's generic FileRegion path no longer over-calls transferTo(). Custom FileRegion implementations that rely on the documented contract work under io_uring with the same semantics they already have under NIO and epoll.

Promote testFileRegionDrainStopsAtCompletion and its OvershootDetectingFileRegion fixture from IoUringSocketFileRegionTest to the SocketFileRegionTest base class so every transport implementation (NIO, epoll, kqueue, io_uring and any future addition) is exercised by the same regression detector. Drop the io_uring-specific guards along the way: the chunk-size and "client must be io_uring" assumptions no longer apply once the test lives in the base class, and all other transports already short-circuit on transferred() >= count() before invoking transferTo(), so they pass the assertion as written.

normanmaurer

Two nits...

Address review nits: route OvershootDetectingFileRegion's count precondition through ObjectUtil.checkInRange(..., 2, Long.MAX_VALUE, "count") rather than an open-coded if/throw, and revert the stray trailing blank line left in IoUringSocketFileRegionTest after the OvershootDetectingFileRegion fixture moved into the base class.

OioByteStreamChannel#doWriteFileRegion drives its drain loop with a locally-tracked bytes-written counter and passes it as the transferTo position, never consulting region.transferred() for termination. The OvershootDetectingFileRegion fixture advances transferred() to count() after writing only one byte -- exactly to exercise the overshoot path on transports that do consult transferred() -- which violates OIO's position invariant and surfaces as a spurious IOException on the OIO permutation in CI. Skip the OIO client via assumeFalse(cc instanceof OioSocketChannel), mirroring the precedent in SocketShutdownOutputBySelfTest. The check runs alongside the existing supportsCustomFileRegion gate so the NIO/epoll/kqueue/io_uring permutations still cover the regression.

LuciferYang · 2026-05-19T10:14:32Z

CI failure traced to the OIO permutation: OioByteStreamChannel#doWriteFileRegion drives its drain loop off a locally-tracked bytes-written counter (passing it as the transferTo position) and never consults region.transferred() for termination. The OvershootDetectingFileRegion fixture advances transferred() to count() after one byte specifically to exercise the overshoot path on transports that do honour transferred() — which conflicts with OIO's position invariant and surfaced as a spurious IOException: transferTo position 1 != transferred() 16 in linux-x86_64-java21 / java11-pooled.

Pushed 088ebed to skip the OIO client via assumeFalse(cc instanceof OioSocketChannel, ...), matching the existing precedent in SocketShutdownOutputBySelfTest. The NIO / epoll / kqueue / io_uring permutations still cover the regression.

normanmaurer · 2026-05-19T13:30:28Z

@LuciferYang thanks!

netty-project-bot · 2026-05-19T13:30:44Z

Could not create auto-port PR.
Got conflicts when cherry-picking onto 4.1.

netty-project-bot · 2026-05-19T13:30:48Z

Could not create auto-port PR.
Got conflicts when cherry-picking onto 5.0.

normanmaurer · 2026-05-19T14:26:52Z

PR for 5.0 #16832

LuciferYang · 2026-05-19T14:29:08Z

Thank you @normanmaurer

#16832) …s count() (#16826) Closes #16825. The generic `FileRegion` fallback added in #16571 drains the region only on `buf.writableBytes() > 0`: ```java while (buf.writableBytes() > 0) { long t = region.transferTo(ch, region.transferred()); if (t <= 0) { break; } } ``` epoll's equivalent at `AbstractEpollStreamChannel#writeFileRegion` guards the call with: ```java if (region.transferred() >= region.count()) { in.remove(); return 0; } ``` so epoll never invokes `transferTo()` past `transferred() == count()`. The io_uring fallback can, which violates the `FileRegion` contract for implementations that lazily emit per-chunk framing — the extra call builds an empty next chunk and ships its framing into the buffer, breaking the receiver. See #16825 for the full diagnosis. Bound the inner drain loop with `region.transferred() < region.count()` in addition to `buf.writableBytes() > 0`, mirroring epoll's short-circuit. Add `IoUringSocketFileRegionTest#testFileRegionDrainStopsAtCompletion` plus an `OvershootDetectingFileRegion` fixture that records every `transferTo()` invocation made past `transferred() == count()`. The fixture writes one byte and advances `transferred` to `count` in one shot, exercising the buggy path (15 overshoots on default chunk size) and the fixed path (0 overshoots) deterministically. A few test-design notes for review: - Skips on non-io_uring client permutations — the drain loop is on the sending side. - Skips when `io.netty.iouring.fileRegionChunkSize` is set to 1, since that leaves no spare capacity for the overshoot to land in. - Asserts on the writer-side counter synchronously after `writeAndFlush(...).sync()`; no time-based wait. - Routes contract violations through `IOException` rather than `AssertionError` so they surface on the write future's cause through the transport's `catch (Exception)` instead of wedging the EventLoop. - `@Timeout(value = 30, unit = SECONDS)` as a safety net for any future regression that wedges `sync()`. io_uring's generic `FileRegion` path stops over-calling `transferTo()`. Custom `FileRegion` implementations that lazily emit per-chunk framing work under io_uring with the same semantics they already have under NIO and epoll. Co-authored-by: YangJie <[email protected]>

LuciferYang force-pushed the fix-iouring-overshoot branch 2 times, most recently from 3a06d4e to fcba21a Compare May 19, 2026 05:34

LuciferYang force-pushed the fix-iouring-overshoot branch from fcba21a to f2f80cd Compare May 19, 2026 05:36

normanmaurer reviewed May 19, 2026

View reviewed changes

Comment thread transport-native-io_uring/src/test/java/io/netty/channel/uring/IoUringSocketFileRegionTest.java Outdated

normanmaurer requested changes May 19, 2026

View reviewed changes

Comment thread testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketFileRegionTest.java Outdated

Comment thread transport-native-io_uring/src/test/java/io/netty/channel/uring/IoUringSocketFileRegionTest.java Outdated

LuciferYang added 2 commits May 19, 2026 17:01

normanmaurer added this to the 4.2.14.Final milestone May 19, 2026

normanmaurer added needs-cherry-pick-4.1 This PR should be cherry-picked to 4.1 once merged. needs-cherry-pick-5.0 This PR should be cherry-picked to 5.0 once merged. labels May 19, 2026

normanmaurer merged commit 9e19320 into netty:4.2 May 19, 2026
19 checks passed

normanmaurer removed the needs-cherry-pick-4.1 This PR should be cherry-picked to 4.1 once merged. label May 19, 2026

normanmaurer mentioned this pull request May 19, 2026

IoUring: Stop generic FileRegion drain loop when transferred() reache… #16832

Merged

LuciferYang mentioned this pull request May 20, 2026

IoUring: Allocate the full chunk buffer for the generic FileRegion fallback #16830

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

IoUring: Stop generic FileRegion drain loop when transferred() reaches count()#16826

IoUring: Stop generic FileRegion drain loop when transferred() reaches count()#16826
normanmaurer merged 4 commits into
netty:4.2from
LuciferYang:fix-iouring-overshoot

LuciferYang commented May 19, 2026

Uh oh!

Uh oh!

normanmaurer left a comment

Uh oh!

Uh oh!

Uh oh!

LuciferYang commented May 19, 2026

Uh oh!

Uh oh!

normanmaurer commented May 19, 2026

Uh oh!

netty-project-bot commented May 19, 2026

Uh oh!

netty-project-bot commented May 19, 2026

Uh oh!

normanmaurer commented May 19, 2026

Uh oh!

LuciferYang commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

LuciferYang commented May 19, 2026

Motivation

Modification

Result

Uh oh!

Uh oh!

normanmaurer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

LuciferYang commented May 19, 2026

Uh oh!

Uh oh!

normanmaurer commented May 19, 2026

Uh oh!

netty-project-bot commented May 19, 2026

Uh oh!

netty-project-bot commented May 19, 2026

Uh oh!

normanmaurer commented May 19, 2026

Uh oh!

LuciferYang commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants