-
Notifications
You must be signed in to change notification settings - Fork 1.2k
server: fix IllegalMonitorStateException on cluster managedstate change #11310
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
server: fix IllegalMonitorStateException on cluster managedstate change #11310
Conversation
Fixes apache#11293 Signed-off-by: Abhishek Kumar <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## 4.19 #11310 +/- ##
=============================================
- Coverage 15.17% 4.28% -10.89%
=============================================
Files 5416 372 -5044
Lines 475621 29738 -445883
Branches 58054 5227 -52827
=============================================
- Hits 72168 1274 -70894
+ Misses 395380 28319 -367061
+ Partials 8073 145 -7928
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
sureshanaparti
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an IllegalMonitorStateException that occurs when updating cluster managed state by replacing an incorrect wait() call with sleep().
- Replace Thread.currentThread().wait() with Thread.sleep() to fix synchronization issue
- Correct improper use of wait() method outside synchronized block
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
Outdated
Show resolved
Hide resolved
DaanHoogland
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A wait should have been on the cluster, my bad. But I think a sleep is very appropriate here.
|
@blueorangutan package |
|
@kiranchavala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14434 |
kiranchavala
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, able to manage and unmage cluster successfully without any issues
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
Outdated
Show resolved
Hide resolved
Pearl1594
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm
…ge (apache#11310) * server: fix IllegalMonitorStateException on cluster managedstate change Fixes apache#11293 * Update server/src/main/java/com/cloud/resource/ResourceManagerImpl.java Signed-off-by: Abhishek Kumar <[email protected]> Co-authored-by: Suresh Kumar Anaparti <[email protected]>
Description
Fixes #11293
The change was introduced in #10518
Thread.currentThread().wait(...) must be called in a synchronized block and on the object.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?