Page History

Wiki MarkupThe {{java.util.Collections}} interface's documentation \[[API 06|AA. Java References#API 06]\] warns about the consequences of synchronizing on any view over a collection object, rather than synchronizing on the collection object itself.The Java Tutorials, Wrapper Implementations [Java Tutorials], 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 Map when iterating over any of its collection views... Failure to follow Collection views rather than synchronizing on the Collection view itself.

Disregarding this advice may result in

...

nondeterministic behavior.

Synchronize on the original Collection object when using synchronization wrappers. This synchronization is necessary to enforce atomicity (CON07-J. Do not assume that a grouping of calls to independently atomic methods is atomic).

This also applies to java.util.Map objects, even though Maps are technically not Collections.

Noncompliant Code Example (collection view)

Any class that uses a collection view rather than the backing collection as the lock object may end up with two distinct locking strategies. When the backing collection is accessible to multiple threads, the class that locked on the collection view has violated the thread-safety properties and is unsafe. Consequently, programs that both require synchronization while iterating over collection views and have accessible backing collections must synchronize on the backing collection; synchronization on the view is a violation of this rule.

Noncompliant Code Example (Collection View)

This noncompliant code example creates a HashMap object and two view objects: a synchronized view of an empty HashMap encapsulated by the mapView field and a set view of the map's keys encapsulated by the setView field. This example synchronizes on setView [Java Tutorials Wiki MarkupThis noncompliant code example incorrectly synchronizes on the {{set}} view of the synchronized map ({{map}}) instead of the collection object \[[Tutorials 08|AA. Java References#Tutorials 08]\].

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

public Map<Integer, String> getMap() {
  return mapView;
}

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

In this example, HashMap provides the backing collection for the synchronized map represented by mapView, which provides the backing collection for setView, as shown in the following figure.

Image Added

The HashMap object is inaccessible, but mapView is accessible via the public getMap() method. Because the synchronized statement uses the intrinsic lock of setView rather than of mapView, another thread can modify the synchronized map and invalidate the k iterator.

Compliant Solution (

...

Collection Lock Object)

This compliant solution synchronizes on the original Collection object map instead of the Collection view set. mapView field rather than on the setView field:

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

public Map<Integer, String> getMap() {
  return mapView;
}

public void doSomething() {
  synchronized (mapmapView) {  // Synchronize on map, rather notthan set
    for (Integer k : setsetView) { 
      // ...
    }
  }
}

This code is compliant because the map's underlying structure cannot be changed during the iteration.

Risk Assessment

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

Related Vulnerabilities

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

References

...

Automated Detection

Some static analysis tools are capable of detecting violations of this rule.

Bibliography

Issue Tracking

||Completed||Priority||Locked||CreatedDate||CompletedDate||Assignee||Name||
|F|M|F|12694529797381270825291208|          |dmohindr| |The title could do away with "still"|
suggested => "HashMap is not accessible, but the Map view is. Because the set view is synchronized instead of the map view, another thread can modify the contents of map and invalidate the k iterator."|

...

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