Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Migration of unmigrated content due to installation of a new plugin

The java.util.Collections interface's documentation [API 2006] 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.

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.

...

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 2008].

Code Block
bgColor#FFcccc
private final Map<Integer, String> mapView =
    Collections.synchronizedMap(new HashMap<Integer, String>());
private final Set<Integer> setView = mapView.keySet();

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

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

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 Figure 10-1: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.

...

This compliant solution synchronizes on the mapView field rather than on the setView field.:

Code Block
bgColor#ccccff
private final Map<Integer, String> mapView =
    Collections.synchronizedMap(new HashMap<Integer, String>());
private final Set<Integer> setView = mapView.keySet();

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

public void doSomething() {
  synchronized (mapView) {  // Synchronize on map, rather than set
    for (Integer k : setView) {
      // ...
    }
  }
}

...

Synchronizing on a collection view instead of the collection object can cause nondeterministic behavior.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

LCK04-J

low

Low

probable

Probable

medium

Medium

P4

L3

Automated Detection

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

ToolVersionCheckerDescription
Parasoft Jtest

Include Page
Parasoft_V
Parasoft_V

CERT.LCK04.SOBCDo not synchronize on a collection view if the backing collection is accessible
ThreadSafe
Include Page
ThreadSafe_V
ThreadSafe_V

CCE_CC_SYNC_ON_VIEW
CCE_CC_ITER_VIEW_NO_LOCK
CCE_CC_ITER_VIEW_BOTH_LOCKS
CCE_CC_ITER_VIEW_WRONG_LOCK

Implemented

 


Bibliography

Issue Tracking

Tasklist
Review List
Review List
sortAscendingfalse
sortBypriority
||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."|

...


...

Image Modified Image Modified Image Modified