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

Race condition in QoSHandler #11763

Closed
arcticmosquito opened this issue May 8, 2024 · 2 comments · Fixed by #11772
Closed

Race condition in QoSHandler #11763

arcticmosquito opened this issue May 8, 2024 · 2 comments · Fixed by #11772
Assignees
@sbordet
Labels
Bug For general bugs on Jetty side

Comments

@arcticmosquito
Copy link

Jetty version(s)
12.0.x

Jetty Environment
core

Java version/vendor (use: java -version)
irrelevant

OS type/version
irrelevant

Description
QoSHandler contains a race between 'expire' (getServer().getScheduler()) and 'resume' threads (getServer().getThreadPool()) , which causes the "resume" thread to busy-loop until another request is suspended, i.e. added to one of the queues, which may never happen.
Consider the following serialization order (Jetty codestyle dropped for brevity):

// 1. resume thread (QoSHandler.java#L291)
int permits = state.getAndIncrement();
if (permits >= 0)
    return;

// 2. expire thread (QoSHandler.java#L361)
boolean removed = queues.get(priority).remove(this);

// 3. resume thread (QoSHandler.java#L299) - now trapped in a busy loop
while (!resumeSuspended())
    Thread.onSpinWait();

How to reproduce?
This might be notoriously hard to reproduce on purpose, but one could reason about the issue without reproduction steps.
@arcticmosquito arcticmosquito added the Bug For general bugs on Jetty side label May 8, 2024
@sbordet sbordet self-assigned this May 8, 2024
@sbordet sbordet moved this to 🏗 In progress in Jetty 12.0.10 (FROZEN) May 8, 2024
@sbordet
Copy link
Contributor

sbordet commented May 9, 2024

@arcticmosquito good catch!

sbordet added a commit that referenced this issue May 9, 2024
@sbordet
Fixes #11763 - Race condition in QoSHandler.
1cd63a5 
Now using a read-write lock to atomically execute expire().
This guarantees that there are no races with resume().

The concurrency between handle() and resume(), which should be the most common case, is handled by atomic data structures.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet linked a pull request May 9, 2024 that will close this issue
Fixes #11763 - Race condition in QoSHandler. #11772
Merged
@arcticmosquito
Copy link
Author

@sbordet thanks for the swift fix

sbordet added a commit that referenced this issue May 14, 2024
@sbordet
Fixes #11763 - Race condition in QoSHandler. (#11772)
a9b2da5 
* Fixes #11763 - Race condition in QoSHandler.

Now using a read-write lock to atomically execute expire().
This guarantees that there are no races with resume().

The concurrency between handle() and resume(), which should be the most common case, is handled by atomic data structures.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Jetty 12.0.10 (FROZEN) May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sbordet sbordet

Labels
Bug For general bugs on Jetty side
Projects
No open projects
Jetty 12.0.10 (FROZEN)
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes #11763 - Race condition in QoSHandler. jetty/jetty.project
2 participants
@sbordet @arcticmosquito