CON06-J. Do not synchronize on a collection view if the backing collection is accessible

The java.util.Collections interface's documentation [[API 06]] warns about the consequences of failing to synchronize on an accessible collection object when iterating over its view:

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.

A class that uses a collection view instead of the backing collection as the lock object, may end up with two different locking strategies. In this case, if the backing collection is accessible to multiple threads, the class is not thread-safe.

Noncompliant Code Example (collection view)

This noncompliant code example creates two views: a synchronized view of an empty map encapsulated by the map field and a set view of the map's keys encapsulated by the set field, and synchronizes on the set view [[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) { 
      // ...
    }
  }
}

In this example, the HashMap provides the backing collection for the Map, which provides the backing collection for Set as shown in the following figure:

The HashMap is inaccessible; however the Map view is accessible. However, because the Set is synchronized instead of the Map, another thread may modify the contents of map and invalidate the k iterator.

Compliant Solution (collection lock object)

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

// 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) { 
      // ...
    }
  }
}

This is compliant because the map's underlying structure cannot be changed when an iteration is in progress.

Risk Assessment

Synchronizing on a collection view instead of the collection object may cause non-deterministic behavior.

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

Issue Tracking

[!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_left.png!]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_up.png!]      [!The CERT Sun Microsystems Secure Coding Standard for Java^button_arrow_right.png!]