Repository navigation
Retransmit lost MAX_DATA frames with the latest connection limit - #100
Merged
Merged
Conversation
ptrd
reviewed
Oct 22, 2026
| } | ||
|
|
||
| private void retransmitMaxData(QuicFrame lostFrame) { | ||
| updateFlowControlLock.lock(); |
Owner
There was a problem hiding this comment.
I don't think the lock is necessary here.
Contributor
Author
There was a problem hiding this comment.
You are right; I have made the necessary correction.
Owner
|
Thanks. Surprised to see there was not retransmit function for this case; you're absolutely right that there ought to be one. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Connection flow-control updates currently use an empty loss callback. If the last MAX_DATA frame is lost and the sender exhausts its old credit, neither side necessarily has more data to consume or send to generate another update, leaving the transfer stalled.
Register a loss callback that retransmits the current connection limit. The callback reads the existing volatile limit without acquiring the update lock. Repeated losses retain the callback, and a lost older update advertises the latest credit rather than a stale limit.
Validation on current master (
101c733d):StreamManagerTesttests pass with JDK 21 on Windows.This PR is independent of receive-window auto-tuning and does not change the configured buffer budget or library version.
Fork CI: the complete
./gradlew buildpassed on Linux / JDK 17 with the upstream workflow and no test exclusions: https://github.com/DeveloperKubilay/kwik/actions/runs/39651941690. Upstream PR checks remain separate.Review follow-up: removed the unnecessary lock from the retransmission callback. All 51
StreamManagerTesttests pass again with JDK 21 on Windows.