Skip to content

Fix Triple flow control deadlock when a message exceeds the stream window - #16443

Open
fudianchn wants to merge 1 commit into
apache:3.3from
fudianchn:fix-triple-flow-control-deadlock
Open

Fix Triple flow control deadlock when a message exceeds the stream window#16443
fudianchn wants to merge 1 commit into
apache:3.3from
fudianchn:fix-triple-flow-control-deadlock

Conversation

@fudianchn

Copy link
Copy Markdown

AI disclosure: this change was prepared with AI coding agents, reviewed and revised line by line by me.

What is the purpose of the change?

Fixes #16427 (case 1).

When a single gRPC message is larger than the HTTP/2 stream-level flow control window, the call deadlocks until it times out: the sender stops once the window is exhausted and waits for WINDOW_UPDATE, while the receiver buffers the partial message and waits for the remaining bytes. Two defects cause this:

  1. LengthFieldStreamingDecoder only reported bytesRead after a full message was assembled (processBody), so bytes buffered inside an incomplete message were never returned to the peer. Now decode() reports the ingested bytes as soon as they are taken off the wire, and the per-message report in processBody() is dropped to avoid double counting. This matches the grpc-java deframer pattern already referenced in the code comment.
  2. consumeBytes() (both NettyH2StreamChannel on the server side and Http2TripleClientStream on the client side) never flushed the WINDOW_UPDATE written by the flow controller: when consumption runs from an executor thread the frame reading loop does not flush it, and flushing the stream channel itself is a no-op for frames written directly to the connection. The frame observed in a frame log sat in the outbound buffer for 5 seconds until a timeout reset flushed it. Both sites now flush the parent channel when consumeBytes wrote an update.

Verified both directions on a real localhost tri connection: with a 64 KiB initial stream window and the ~92 KiB message from the issue, the frame log shows the request and the response each delivered as 65535 + 29003 bytes with WINDOW_UPDATE flowing in between, instead of a deadlock until the 5s deadline. The second failure mode from the issue (window exceeded against a PHP gRPC peer) needs a peer that keeps sending beyond the advertised window and did not reproduce between two Netty endpoints, so it is not addressed here.

Tests: TripleFlowControlTest (unary and server-stream against a real localhost endpoint, red before the change: DEADLINE_EXCEEDED after 5s, green after), GrpcStreamingDecoderTest.reportsBytesReadForIncompleteMessage (partial message reports its bytes, red before: expected 14 but was 0). mvn -pl dubbo-remoting/dubbo-remoting-http12,dubbo-rpc/dubbo-rpc-triple -am test passes.

Checklist

  • Make sure there is a GitHub_issue field for the change.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
  • Make sure gitHub actions can pass. Why the workflow is failing and how to fix it?

…ndow

A single gRPC message larger than the HTTP/2 stream-level flow control
window can never complete: LengthFieldStreamingDecoder only reported
bytesRead after a full message was assembled, so the partially received
bytes were never returned to the peer and both sides waited on each
other until the call timed out (issue apache#16427, case 1).

Report the ingested bytes in decode() as soon as they are taken off the
wire, even if the message they belong to is still incomplete, and drop
the per-message report in processBody() to avoid double counting.

Also flush the parent channel after consumeBytes() writes a
WINDOW_UPDATE: when consumption runs from an executor thread the frame
reading loop does not flush it, and flushing the stream channel itself
is a no-op for frames written directly to the connection, so the update
sat in the outbound buffer and the peer never saw the returned window.

Fixes apache#16427

Signed-off-by: 付典 <fudianchn@gmail.com>
@codecov-commenter

codecov-commenter commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.47059% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.94%. Comparing base (5553cb7) to head (91a4205).

Files with missing lines Patch % Lines
...ng/http12/message/LengthFieldStreamingDecoder.java 71.42% 2 Missing ⚠️
...emoting/http12/netty4/h2/NettyH2StreamChannel.java 80.00% 1 Missing ⚠️
...rotocol/tri/h12/http2/Http2TripleClientStream.java 80.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                3.3   #16443      +/-   ##
============================================
+ Coverage     60.90%   60.94%   +0.04%     
- Complexity    11765    11766       +1     
============================================
  Files          1953     1953              
  Lines         89271    89281      +10     
  Branches      13473    13474       +1     
============================================
+ Hits          54367    54411      +44     
+ Misses        29321    29295      -26     
+ Partials       5583     5575       -8     
Flag Coverage Δ
integration-tests-java21 32.14% <47.05%> (+<0.01%) ⬆️
integration-tests-java8 32.21% <47.05%> (+<0.01%) ⬆️
samples-tests-java21 32.14% <35.29%> (-0.02%) ⬇️
samples-tests-java8 29.85% <35.29%> (+0.03%) ⬆️
unit-tests-java11 59.21% <76.47%> (+0.04%) ⬆️
unit-tests-java17 58.68% <76.47%> (+0.03%) ⬆️
unit-tests-java21 58.69% <76.47%> (+0.05%) ⬆️
unit-tests-java25 58.64% <76.47%> (+0.03%) ⬆️
unit-tests-java8 59.20% <76.47%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

[Bug] Triple Java client HTTP/2 flow-control failure with PHP gRPC unary response larger than 64 KiB

2 participants