Page History

{quote}

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.

{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 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}} \[[Tutorials 2008|AA. References#Tutorials 08]\].

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

!con06-j-backing-collection.JPG!

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.


h2. Compliant Solution (Collection Lock Object)

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

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

|| Rule || Severity || Likelihood || Remediation Cost || Priority || Level ||
| LCK04-J  | low | probable | medium | {color:green}{*}P4{*}{color} | {color:green}{*}L3{*}{color} |


h2. Bibliography

| \[[API 2006|AA. References#API 06]\] | Class Collections |
| \[[Tutorials 2008|AA. References#Tutorials 08]\] | [Wrapper Implementations|http://java.sun.com/docs/books/tutorial/collections/implementations/wrapper.html] |


h2. Issue Tracking 

{tasklist:Review List|sortAscending=false|sortBy=priority}
||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."|
{tasklist}


----
[!The CERT Oracle Secure Coding Standard for Java^button_arrow_left.png!|LCK03-J. Do not synchronize on the intrinsic locks of high-level concurrency objects]      [!The CERT Oracle Secure Coding Standard for Java^button_arrow_up.png!|08. Locking (LCK)]      [!The CERT Oracle Secure Coding Standard for Java^button_arrow_right.png!|LCK05-J. Synchronize access to static fields that can be modified by untrusted code]