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
[JENKINS-24763] Prevent thread starvation in OldDataMonitor by reducing scope of synchronization #1402
Conversation
…ng scope of synchronization
@jglick @olivergondza, I reworked this to reduce the scope of synchronization and added a test which reproduces the issue |
// test will hang here due to JENKINS-24763 | ||
OldDataMonitor.changeListener | ||
.onChange(new Saveable() {public void save() throws IOException {}}, | ||
new XmlFile(File.createTempFile("somexmlfile","xml"))); |
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.
No indication what program this is from, and no cleanup.
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.
Resolved in 2e446fd
LGTM |
|
||
synchronized (this) { | ||
for (SaveableReference ref : removed) { | ||
data.remove(ref); |
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.
data.keySet().removeAll(removed)
probably suffices, but not sure.
@jglick, if this looks OK, can you merge? |
Supercedes #1399