CON40-J. Synchronize on the collection object instead of the collection view

The java.util.Collections interface's documentation [[API 06]] warns about the consequences of failing to synchronize on the collection object:

It is imperative that the user manually synchronize on the returned map when iterating over any of its collection views... Failure to follow this advice may result in non-deterministic behavior.

When using synchronization wrappers, the synchronization object should be the Collection object. The synchronization is necessary to enforce atomicity (CON07-J. Do not assume that a grouping of calls to independently atomic methods is atomic).

Noncompliant Code Example (collection view)

This noncompliant code example incorrectly synchronizes on the set view of the synchronized map (map) instead of the collection object [[Tutorials 08]].

// map has package-private accessibility
final Map<Integer, String> map = Collections.synchronizedMap(new HashMap<Integer, String>());
private final Set<Integer> set = map.keySet();

public void doSomething() {
  synchronized(set) {  // Incorrectly synchronizes on set
    for (Integer k : set) { 
      // ...
    }
  }
}

Compliant Solution (collection lock object)

This compliant solution synchronizes on the Collection object map instead of the Collection view set.

// map has package-private accessibility
final Map<Integer, String> map = Collections.synchronizedMap(new HashMap<Integer, String>());
private final Set<Integer> set = map.keySet();

public void doSomething() {
  synchronized(map) {  // Synchronize on map, not set
    for (Integer k : set) { 
      // ...
    }
  }
}

Risk Assessment

Synchronizing on a collection view instead of the collection object provides a false sense of thread safety and may cause non-deterministic behavior.

Automated Detection

The following table summarizes the examples flagged as violations by FindBugs:

The following table summarizes the examples flagged as violations by SureLogic Flashlight:

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

References

[[API 06]] Class Collections
[[Tutorials 08]] Wrapper Implementations

VOID CON00-J. Synchronize access to shared mutable variables      11. Concurrency (CON)      CON03-J. Do not use background threads during class initialization