Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

Wiki Markup
The {{java.util.Collections}} interface's documentation \[[API 06|AA. Java References#API 06]\] warns about the consequences of failing to synchronize on the collection object:

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.

When using synchronization wrappers, the synchronization object should be the Collection object. The synchronization is necessary to enforce atomicity (CON07-J. Do not assume that a grouping of calls to independently atomic methods is atomic).

Noncompliant Code Example (collection view)

Wiki Markup
This 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]\].

Code Block
bgColor#FFcccc
// 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) { 
      // ...
    }
  }
}

Wiki Markup
When using synchronization wrappers, the synchronization object should be the {{Collection}} object. The synchronization is necessary to enforce atomicity ([CON07-J. Do not assume that a grouping of calls to independently atomic methods is atomic]). This noncompliant code example demonstrates inappropriate synchronization resulting from locking on a Collection view instead of the Collection object itself \[[Tutorials 08|AA. Java References#Tutorials 08]\]. 

Wiki Markup
The {{java.util.Collections}} interface's documentation \[[API 06|AA. Java References#API 06]\] warns about the consequences of following this practice:

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.

Compliant Solution (collection lock object)

...

Code Block
bgColor#ccccff
// 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) { 
      // ...
    }
  }
}

Risk Assessment

Synchronizing on a collection view instead of the collection object provides a false sense of thread safety and may cause non-deterministic behavior.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

CON40- J

medium

probable

medium

P8

L2

Automated Detection

The following table summarizes the examples flagged as violations by FindBugs:

Noncompliant Code Example

Flagged

Checker

Message

Collection view

No

n/a

The following table summarizes the examples flagged as violations by SureLogic Flashlight:

Noncompliant Code Example

Flagged

Message

Collection view

No

No obvious issues

Related Vulnerabilities

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

References

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

...

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