Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: edits

...

When explicitly constructed, an Integer object has a unique reference and its own intrinsic lock that is not shared with other Integer objects or boxed integers having the same value. While this is an acceptable solution, it may cause maintenance problems. It is always better A more appropriate solution is to synchronize on an internal private final raw lock Object as described next.

Compliant Solution (internal private final

...

lock Object)

This compliant solution uses an internal private final lock object. This is one of the few cases where a raw Object is useful.

...

Consequently, an interned String object behaves like a global variable in the Java Virtual Machine (JVM). As demonstrated in this noncompliant code example, even if every instance of an object maintains its own field lock, the field points to a common String constant in the JVM. Trusted code that locks on the same String constant renders all synchronization attempts inadequate. Similarly, hostile code from any other package can exploit this vulnerability if the class is accessible.

...

A String instance differs from a String literal. The instance has a unique reference and its own intrinsic lock that is not shared by other string objects object instances or literals. A more suitable approach is to use an internal private lock as discussed earlier.

...

This does not mean that a subclass using getClass() can only synchronize on the Class object of the base class. In fact, it will lock on its own Class object, which may or may not be what the programmer had in mind. The intent should be appropriately clearly documented or annotated.

Compliant Solution (class name qualification)

...

The class object being synchronized should not be accessible to hostile code. If the class is package-private, then external callers from other packages may not access the Class object, ensuring its trustworthiness as an intrinsic lock object. For more information, see CON04-J. Synchronize using an internal private final lock object.

...

Instead of using the intrinsic locks of objects that implement the Lock interface, including such as ReentrantLock, use the lock() and unlock() methods provided by the Lock interface.

Code Block
bgColor#ccccff
private final Lock lock = new ReentrantLock();

public void doSomething() {
  lock.lock();
  try {
    // ...
  } finally {
    lock.unlock();
  }
}

If there is no real need requirement of using the advanced functionality of the dynamic locking utilities of package java.util.concurrent, prefer using the Executor framework or other concurrency primitives such as synchronization and atomic classes.

...

This noncompliant code example synchronizes on the view of a synchronized map.

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 mustshould 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}} interfaces' documentation \[[API 06|AA. Java References#API 06]\] supports statesthis recommendation:

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.

...

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

Risk Assessment

...