Skip to content

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

Merged
normanmaurer merged 1 commit into
5.0from
fr_uring5
May 19, 2026
Merged

IoUring: Stop generic FileRegion drain loop when transferred() reache…#16832
normanmaurer merged 1 commit into
5.0from
fr_uring5

Conversation

@normanmaurer
Copy link
Copy Markdown
Member

…s count() (#16826)

Closes #16825.

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.

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.

…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.
@normanmaurer normanmaurer merged commit bd17039 into 5.0 May 19, 2026
13 checks passed
@normanmaurer normanmaurer deleted the fr_uring5 branch May 19, 2026 17:44
@normanmaurer normanmaurer added this to the 5.0.0.Final milestone May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants