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
when iterating over any of itsCollection
views rather than synchronizing on theCollection
view itself.
Disregarding this advice may result in nondeterministic behavior.
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].
Code Block | ||
---|---|---|
| ||
private final Map<Integer, String> mapView =
| ||
Wiki Markup | ||
The {{java.util.Collections}} interface's documentation \[[API 2006|AA. Java References#API 06]\] warns about the consequences of failing to synchronize on an accessible collection object when iterating over its view: {quote} 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. {quote} 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. {mc} I don't see the point of this statement To make a group of statements atomic, synchronize on the original collection object when using synchronization wrappers.([CON07-J. Do not assume that a group of calls to independently atomic methods is atomic]). {mc} h2. Noncompliant Code Example (Collection View) This noncompliant code example creates two views: a synchronized view of an empty {{HashMap}} encapsulated by the {{map}} field and a set view of the map's keys encapsulated by the {{set}} field. This example synchronizes on the {{set}} view \[[Tutorials 2008|AA. Java References#Tutorials 08]\]. {code:bgColor=#FFcccc} // map has package-private accessibility final Map<Integer, String> map = 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) { // ... } } } {code} |
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.
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 mapView
field rather than on the setView
field:
Code Block | ||
---|---|---|
| ||
private for {{Map}}, which provides the backing collection for {{Set}}, as shown in the following figure: !con06-j-backing-collection.JPG! {{HashMap}} is not accessible, but the {{Map}} view is. Because the {{Set}} view is synchronized instead of {{Map}} view, another thread can modify the contents of {{map}} and invalidate the {{k}} iterator. h2. Compliant Solution (Collection Lock Object) This compliant solution synchronizes on the {{map}} view instead of the {{set}} view. {code:bgColor=#ccccff} // 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) { // ... } } } {code} |
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 can cause nondeterministic behavior.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
LCK04-J | Low | Probable | Medium | P4 | L3 |
Automated Detection
Some static analysis tools are capable of detecting violations of this rule.
Tool | Version | Checker | Description | ||||||
---|---|---|---|---|---|---|---|---|---|
Parasoft Jtest |
| CERT.LCK04.SOBC | Do not synchronize on a collection view if the backing collection is accessible | ||||||
ThreadSafe |
| CCE_CC_SYNC_ON_VIEW | Implemented |
Bibliography
Issue Tracking
Tasklist | ||||||||
---|---|---|---|---|---|---|---|---|
| ||||||||
||Completed||Priority||Locked||CreatedDate||CompletedDate||Assignee||Name||
|F|M|F|1270825291208| |dmohindr|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."|
|
...