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

Fixes #6072 - jetty server high CPU when client sends TLS record length > 17408 (jetty-10.0.x) #6074

Merged
merged 2 commits into from Mar 22, 2021

Conversation

sbordet
Copy link
Contributor

@sbordet sbordet commented Mar 18, 2021

…408.

Avoid spinning if the input buffer is full.

Signed-off-by: Simone Bordet simone.bordet@gmail.com
Co-authored-by: Joakim Erdfelt joakim.erdfelt@gmail.com

…408.

Avoid spinning if the input buffer is full.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Co-authored-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@sbordet sbordet requested a review from joakime March 18, 2021 15:07
@sbordet
Copy link
Contributor Author

sbordet commented Mar 18, 2021

Forward port of #6073

@joakime joakime changed the title Fixes #6072 - jetty server high CPU when client send data length > 17… Fixes #6072 - jetty server high CPU when client send data length > 17408 Mar 18, 2021
@joakime joakime added the Bug For general bugs on Jetty side label Mar 18, 2021
@joakime joakime added this to In progress in Jetty 10.0.2/11.0.2 via automation Mar 18, 2021
@joakime joakime changed the title Fixes #6072 - jetty server high CPU when client send data length > 17408 Fixes #6072 - jetty server high CPU when client send data length > 17408 (jetty-10.0.x) Mar 18, 2021
@joakime joakime changed the title Fixes #6072 - jetty server high CPU when client send data length > 17408 (jetty-10.0.x) Fixes #6072 - jetty server high CPU when client sends TLS record length > 17408 (jetty-10.0.x) Mar 18, 2021
Jetty 10.0.2/11.0.2 automation moved this from In progress to Reviewer approved Mar 19, 2021
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

changed my mind. I think we should check space on any 0 fill

Jetty 10.0.2/11.0.2 automation moved this from Reviewer approved to Review in progress Mar 22, 2021
…408.

Updates after review.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet requested a review from gregw March 22, 2021 09:48
Jetty 10.0.2/11.0.2 automation moved this from Review in progress to Reviewer approved Mar 22, 2021

// Sleep to see if the server spins.
Thread.sleep(1000);
assertThat(unwraps.get(), lessThan(128L));
Copy link
Contributor

Choose a reason for hiding this comment

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

I would move the assertion out of the try-with-resource block as the IO.readBytes call already would wait 1s if this bug is triggered.

@sbordet sbordet merged commit 039c738 into jetty-10.0.x Mar 22, 2021
Jetty 10.0.2/11.0.2 automation moved this from Reviewer approved to Done Mar 22, 2021
@sbordet sbordet deleted the jetty-10.0.x-6072-ssl-cpuspin branch March 22, 2021 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

High CPU usage when TLS client sends TLS Record data exceeding length 17408
4 participants