-
Notifications
You must be signed in to change notification settings - Fork 317
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
fix: limiter deadlock while trying to notify a dynamic priority item #3056
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## release/1.6.x #3056 +/- ##
================================================
Coverage ? 52.86%
================================================
Files ? 338
Lines ? 52135
Branches ? 0
================================================
Hits ? 27562
Misses ? 22967
Partials ? 1606 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Can you include parts of the trace that indicate the problem? It would be easier to understand the problem this way |
Can you add a benchmark test that simulates a case of high contention? |
There is a benchmark test already here, but didn't really managed to manifest the issue :( |
|
Description
In case of high contention, a deadlock was possible if the limiter was trying to notify a dynamic priority item which in turn was trying to increase its priority in the queue.
The limiter now releases the lock earlier, just before notifying the item's channel. The (removed) item will no longer block trying to increase its priority in the queue, it will execute this no-op and then get properly notified.
Notion Ticket
Link
Security