| synchronization on Boolean fields in Eclipse DTP project [message #597466] |
Mon, 03 May 2010 21:34 |
 Bill Pugh
Messages: 3
Registered: July 2009
Location: Univ. of Maryland |
Junior Member |
|
|
Reviewing warnings generated by FindBugs on the Eclipse DTP project, we found 522 cases of synchronization on Boolean values, such as in DerbyCatalogSchema:
private Boolean tablesLoaded = Boolean.FALSE;
private Boolean routinesLoaded = Boolean.FALSE;
private Boolean udtsLoaded = Boolean.FALSE;
public void refresh() {
synchronized (tablesLoaded) {
if (tablesLoaded.booleanValue()) {
tablesLoaded = Boolean.FALSE;
}
}
synchronized (routinesLoaded) {
if (routinesLoaded.booleanValue()) {
routinesLoaded = Boolean.FALSE;
}
}
synchronized (udtsLoaded) {
if (udtsLoaded.booleanValue()) {
udtsLoaded = Boolean.FALSE;
}
}
RefreshManager.getInstance().referesh(this);
}
You can't synchronize on a field, only on the contents of a field. Since there are only two Boolean objects, all 522 of these synchronization locations are synchronizing on the same two objects, leading to unintended contention. Also, if the field is changed, a second attempt to synchronize on the field will be locking on the other Boolean value, and thus won't provide the intended mutual exclusion. For example, in JDBCStructuredUDT:
public void refresh() {
synchronized (attributesLoaded) {
if (attributesLoaded.booleanValue()) {
attributesLoaded = Boolean.FALSE;
getParameterLoader().clearAttributeDefinitions(
super.getAttributes());
}
}
synchronized (superLoaded) {
if (superLoaded.booleanValue()) {
superLoaded = Boolean.FALSE;
setSuper(null);
}
}
RefreshManager.getInstance().referesh(this);
}
public EList getAttributes() {
synchronized (attributesLoaded) {
if (!attributesLoaded.booleanValue())
loadAttributes();
}
return super.getAttributes();
}
one thread could call refresh, synchronize on Boolean.TRUE, and set attributesLoaded to Boolean.FALSE, and then get context switched out (while still holding the lock on Boolean.TRUE). At that point, another thread could call getAttributes(), synchronize on Boolean.FALSE, and call loadAttributes(), while the other thread is still in the middle of the refresh call.
See CERT coding rule https://www.securecoding.cert.org/confluence/display/java/CO N08-J.+Do+not+synchronize+on+objects+that+may+be+reused for more information on this bug pattern.
To review all of the issues found by FindBugs in the Eclipse DTP project, you can use Java WebStart to review and comment on the http://findbugs.cs.umd.edu/cloud/eclipse-dtp.jnlp
--
Bill Pugh
Professor, Univ. of Maryland
Lead, FindBugs project
Bill Pugh
Professor, Univ. of Maryland
Lead, FindBugs project
|
|
|
Powered by
FUDForum. Page generated in 0.02706 seconds
] 
Search
Help
Register
Login
Home
