Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improve cleanup of deflater/inflater pools for PerMessageDeflateExtension #8134

Merged
merged 4 commits into from
Jun 9, 2022
Merged

Improve cleanup of deflater/inflater pools for PerMessageDeflateExtension #8134

merged 4 commits into from
Jun 9, 2022

Conversation

lachlan-roberts
Copy link
Contributor

  • Make Pool class available from CompressionPool via JMX.
  • Add close method to clean up resources in Extensions.
  • Close on PermessageDeflateExtension should release deflaters.
  • Avoid using static exceptions from FrameFlusher.

lorban
lorban previously requested changes Jun 7, 2022
View reviewed changes
Copy link
Contributor

@lorban lorban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be wise to use a non-stack-filling exception in close(). Otherwise LGTM.

...mmon/src/main/java/org/eclipse/jetty/websocket/core/internal/PerMessageDeflateExtension.java Outdated Show resolved Hide resolved
@joakime
Copy link
Contributor

joakime commented Jun 8, 2022

Do we want this in Jetty 10.0.10 release?

@lachlan-roberts
Copy link
Contributor Author

@joakime yes this should be in the 10.0.10 release.

@joakime joakime added the Sponsored This issue affects a user with a commercial support agreement label Jun 8, 2022
@sbordet sbordet self-requested a review June 8, 2022 22:09
@lachlan-roberts lachlan-roberts dismissed lorban’s stale review June 8, 2022 22:13

Comment addressed.

@lachlan-roberts lachlan-roberts merged commit b1c19c0 into jetty-10.0.x Jun 9, 2022
@lachlan-roberts lachlan-roberts deleted the jetty-10.0.x-websocketPermessageDeflatePools branch June 9, 2022 23:43
gregw
gregw reviewed Jun 10, 2022
View reviewed changes
...socket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/FrameFlusher.java
@@ -44,7 +44,6 @@ public class FrameFlusher extends IteratingCallback
{
public static final Frame FLUSH_FRAME = new Frame(OpCode.BINARY);
private static final Logger LOG = LoggerFactory.getLogger(FrameFlusher.class);
private static final Throwable CLOSED_CHANNEL = new ClosedChannelException();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lachlan-roberts as discussed on chat.... I think we can keep this static sentinael and just use the constructor that avoids suppressed exceptions and a stacktrace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregw gregw gregw left review comments

@sbordet sbordet sbordet approved these changes

@lorban lorban Awaiting requested review from lorban

Assignees
No one assigned
Labels
Sponsored This issue affects a user with a commercial support agreement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lachlan-roberts @joakime @gregw @sbordet @lorban