-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Improve ssl buffers handling #8165
Conversation
Signed-off-by: Ludovic Orban <[email protected]>
Signed-off-by: Ludovic Orban <[email protected]>
Signed-off-by: Ludovic Orban <[email protected]>
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.
I don't like the "clear to force a release later" pattern. Instead just have clearly named methods that release if empty vs always releases:
releaseEmptyInputBuffers
;releaseInputBuffers
releaseInputBuffersIfEmpty
;clearAndReleaseInputBuffers
But ultimately, we should not need comments like "clearing so a later release will take effect". Instead we should just release immediately in those locations - the subsequent final releases are already null check protected.
jetty-io/src/main/java/org/eclipse/jetty/io/ArrayRetainableByteBufferPool.java
Show resolved
Hide resolved
jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java
Outdated
Show resolved
Hide resolved
jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java
Outdated
Show resolved
Hide resolved
jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java
Outdated
Show resolved
Hide resolved
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.
I agree with @gregw comments about having a discard*Buffer
that does both clearing and releasing.
jetty-io/src/main/java/org/eclipse/jetty/io/ArrayRetainableByteBufferPool.java
Show resolved
Hide resolved
jetty-server/src/main/config/modules/retainablebytebufferpool.mod
Outdated
Show resolved
Hide resolved
jetty-server/src/main/config/modules/retainablebytebufferpool.mod
Outdated
Show resolved
Hide resolved
jetty-server/src/main/config/modules/retainablebytebufferpool.mod
Outdated
Show resolved
Hide resolved
Signed-off-by: Ludovic Orban <[email protected]>
Signed-off-by: Ludovic Orban <[email protected]>
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
Signed-off-by: Ludovic Orban <[email protected]>
Solves #8161