New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Guard against re-entrancy issues while draining AbstractCoalescingBuf… #10294
Merged
Conversation
This file contains 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
This was referenced May 14, 2020
/cc @Entea |
normanmaurer
requested review from
Scottmitch,
carl-mastrangelo,
ejona86 and
franz1981
May 14, 2020 10:00
…ferQueue Motivation: AbstractCoalescingBufferQueue had a bug which could lead to an empty queue while still report bytes left. This was due the fact that we decremented the pending bytes before draining the queue one-by-one. The problem here is that while the queue is drained we may notify the promise which may add again buffers to the queue for which we never decrement the bytes while we drain these Modifications: - Decrement the pending bytes every time we drain a buffer from the queue - Add unit tests Result: Fixes #10286
normanmaurer
force-pushed
the
coalescing_buffer_reentrancy
branch
from
May 14, 2020 10:04
76d1e32
to
462dbb8
Compare
progulin
reviewed
May 14, 2020
transport/src/main/java/io/netty/channel/AbstractCoalescingBufferQueue.java
Show resolved
Hide resolved
@netty-bot test this please |
Scottmitch
approved these changes
May 14, 2020
transport/src/main/java/io/netty/channel/AbstractCoalescingBufferQueue.java
Show resolved
Hide resolved
ejona86
approved these changes
May 14, 2020
carl-mastrangelo
approved these changes
May 14, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
normanmaurer
added a commit
that referenced
this pull request
May 15, 2020
…ferQueue (#10294) Motivation: AbstractCoalescingBufferQueue had a bug which could lead to an empty queue while still report bytes left. This was due the fact that we decremented the pending bytes before draining the queue one-by-one. The problem here is that while the queue is drained we may notify the promise which may add again buffers to the queue for which we never decrement the bytes while we drain these Modifications: - Decrement the pending bytes every time we drain a buffer from the queue - Add unit tests Result: Fixes #10286
Kvicii
pushed a commit
to Kvicii/netty
that referenced
this pull request
May 19, 2020
* '4.1' of github.com:netty/netty: Use allocation free algorithm to detect CNAME cache loops (netty#10291) Do not require BoringSSL for testSessionTicketsWithTLSv12AndNoKey (netty#10301) Check if SSL pointer was freed before using it in RefereceCountedOpenSslEngine in all cases (netty#10299) Include aarch64 packages in netty-bom (netty#10292) Make ReferenceCountedOpenSslContext.setUseTasks public (netty#10289) Respect jdk.tls.client.enableSessionTicketExtension and jdk.tls.server.enableSessionTicketExtension when using native SSL impl (netty#10296) Guard against re-entrancy issues while draining AbstractCoalescingBufferQueue (netty#10294) Add DNS client examples for run-example.sh (netty#10283) Allow to have the session tickets automatically managed by the native… (netty#10280) Fix a potential fd leak in AbstractDiskHttpData.getChunk (netty#10270) Fix classifier for aarch64 (netty#10279) Fix regression in HttpPostStandardRequestDecoder to always decode + to whitespace (netty#10285) [maven-release-plugin] prepare for next development iteration [maven-release-plugin] prepare release netty-4.1.50.Final Use correct JDK 13 version (netty#10276) OpenSslSession.getLocalCertificates() and getLocalPrincipal() must r… (netty#10275) Update Java versions (netty#10273) Select correct nameserver for CNAME (netty#10272)
ihanyong
pushed a commit
to ihanyong/netty
that referenced
this pull request
Jul 31, 2020
…ferQueue (netty#10294) Motivation: AbstractCoalescingBufferQueue had a bug which could lead to an empty queue while still report bytes left. This was due the fact that we decremented the pending bytes before draining the queue one-by-one. The problem here is that while the queue is drained we may notify the promise which may add again buffers to the queue for which we never decrement the bytes while we drain these Modifications: - Decrement the pending bytes every time we drain a buffer from the queue - Add unit tests Result: Fixes netty#10286
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.
…ferQueue
Motivation:
AbstractCoalescingBufferQueue had a bug which could lead to an empty queue while still report bytes left. This was due the fact that we decremented the pending bytes before draining the queue one-by-one. The problem here is that while the queue is drained we may notify the promise which may add again buffers to the queue for which we never decrement the bytes while we drain these
Modifications:
Result:
Fixes #10286