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.

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 cleanup of deflater/inflater pools for PerMessageDeflateExtension #8134

Merged

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.

…sion

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lorban
lorban previously requested changes Jun 7, 2022
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.

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@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.

@lachlan-roberts lachlan-roberts added this to In progress in Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶 via automation Jun 8, 2022
@joakime joakime moved this from In progress to Review in progress in Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶 Jun 8, 2022
@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
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts lachlan-roberts dismissed lorban’s stale review June 8, 2022 22:13

Comment addressed.

Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶 automation moved this from Review in progress to Reviewer approved Jun 8, 2022
@lachlan-roberts lachlan-roberts merged commit b1c19c0 into jetty-10.0.x Jun 9, 2022
Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶 automation moved this from Reviewer approved to Done Jun 9, 2022
@lachlan-roberts lachlan-roberts deleted the jetty-10.0.x-websocketPermessageDeflatePools branch June 9, 2022 23:43
@@ -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
Labels
Sponsored This issue affects a user with a commercial support agreement
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants