Page History

...

Boxed types are allowed to use the same instance for a range of integer values and consequently, suffer from the same problems problem as Boolean constants. If the primitive can be represented as a byte, the wrapper object is reused. Note that the boxed Integer primitive is shared and not the Integer object (new Integer(value)) itself. In general, holding a lock on any data structure type that contains a boxed value is insecure.

...

When explicitly constructed, an Integer object has a unique reference and its own intrinsic lock that is not shared by 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 to synchronize on a private final raw Object as described next.

...

According to the Java API \[[API 06|AA. Java References#API 06]\], class {{java.lang.String}} documentation:

When the intern() method is invoked, if the pool already contains a string equal to this String object as determined by the equals(Object) method, then the string from the pool is returned. Otherwise, this String object is added to the pool and a reference to this String object is returned.

Consequently, an interned String object behaves like a global variable in the 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. LikewiseSimilarly, hostile code from any other package can exploit this vulnerability.

...

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 or literals. A more suitable approach is to use the private final internal raw Object discussed earlier.

...

public void doSomething() {
  synchronized(Class.forName("SuperclassName")) { 
    // ... 
  }
}

Again, the The class object being synchronized must not be accessible to hostile code, as discussed in the previous compliant solution. Furthermore, care must be taken so that untrusted inputs are not accepted as arguments while loading classes using Class.forname() (see SEC05-J. Do not expose standard APIs that use the immediate caller's class loader instance to untrusted code for more information).

...

Similarly, it is inappropriate to lock on an object of a class that implements either the Lock or Condition interface (or both) of package java.util.concurrent.locks. Using intrinsic locks of these classes is a questionable practice even though the code may appear to function correctly. This problem usually comes up in practice when refactoring from intrinsic locking to the java.util.concurrent dynamic locking utilities.

...

private final Lock lock = new ReentrantLock();

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

It is recommended to not use the intrinsic locks of objects of classes that implement Lock or Condition interfaces. If there is no real need 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.

...

// ...
Map<Integer, String> map = Collections.synchronizedMap(new HashMap<Integer, String>());

public void doSomething() {
  synchronized(map) {  // Synchronize on map, not set
    for(Integer k : map) { 
      // Do something  ...
    }
  }
}

Risk Assessment

Synchronizing on an inappropriate field can provide a false sense of thread safety and result in non-deterministic behavior.

...