Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Eclipse Projects » DTP » synchronization on Boolean fields in Eclipse DTP project
synchronization on Boolean fields in Eclipse DTP project [message #531150] Mon, 03 May 2010 17:34
Bill Pugh is currently offline 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 CON08-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 FindBugs Eclipse DTP community review


Bill Pugh
Professor, Univ. of Maryland
Lead, FindBugs project
Previous Topic:Problems executing a script with multiple insert
Next Topic:synchronization on Boolean fields in Eclipse DTP project
Goto Forum:
  


Current Time: Wed Apr 16 18:47:01 EDT 2014

Powered by FUDForum. Page generated in 0.01943 seconds