Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: I'm happy with this rule, but I suspect Dhruv won't be

...

Wiki Markup
This technique is also called client-side locking \[[Goetz 06|AA. Java References#Goetz 06]\], because the class holds a lock on an object that presumably might be accessible to other classes. Client-side locking shouldis not bealways usedan when the underlying class does not commit to its locking strategy. For more informationappropriate strategy; see [CON31-J. Avoid client-side locking when using classes that do not commit to their locking strategy] for more information. 

Wiki Markup
Although expensive in terms of performance, the {{CopyOnWriteArrayList}} and {{CopyOnWriteArraySet}} classes are sometimes used to create copies of the core {{Collection}} so that iterators do not fail with a runtime exception when some data in the {{Collection}} is modified. However, any updates to the {{Collection}} are not immediately visible to other threads. Consequently, the use of these classes is limited to boosting performance in code where the writes are fewer (or non-existent) as compared to the reads  \[[JavaThreads 04|AA. Java References#JavaThreads 04]\]. In most other cases they must be avoided (see [MSC13-J. Do not modify the underlying collection when an iteration is in progress] for details on using these classes).    

Noncompliant Code Example (synchronizedMap)

Wiki MarkupThis code does not violate CON40-J. Do not synchronize on a collection view if the backing collection is still accessible, because while it does synchronize on a collectoin view (the synchronizedList), the backing collection is not accessible, and hence cannot be modified by any code.

Noncompliant Code Example (synchronizedMap)

Wiki Markup
This noncompliant code example defines a class {{KeyedCounter}} which is not thread-safe. Even though the {{HashMap}} is wrapped in a synchronized {{Map}}, the  noncompliant code example defines a class {{KeyedCounter}} which is not thread-safe. Even though the {{HashMap}} is wrapped in a synchronized {{Map}}, the overall increment operation is not atomic. \[[Lee 09|AA. Java References#Lee 09]\]   

Code Block
bgColor#FFCCCC
final class KeyedCounter {
  private final Map<String, Integer> map =
    Collections.synchronizedMap(new HashMap<String, Integer>());

  public void increment(String key) {
    Integer old = map.get(key);
    int valueoldValue = (old == null) ? 10 : old.intValue() + 1;
    if map.put(key, value);oldValue == Integer.MAX_VALUE) {    
  }

    publicthrow Integernew getCount(String key) {ArithmeticException("Out of range");
    }
    return map.getput( key, value + 1);
  }
}

  public Integer getCount(String key) {
    return map.get(key);
  }
}

Note that while this code is thread-unsafe, it at least prevents integer overflow when incrementing the map values, as mandated by The integer overflow check that should be used before incrementing has been omitted for brevity. To prevent overflow, the caller must ensure that the increment() method is called no more than Integer.MAX_VALUE times for any key. Refer to INT00-J. Perform explicit range checking to ensure integer operations do not overflow for more information.

Compliant Solution (synchronized blocks)

...

Code Block
bgColor#ccccff
final class KeyedCounter {
  private final Map<String, Integer> map = new HashMap<String, Integer>(); 
  private final Object lock = new Object();

  public void increment(String key) {
    synchronized (lock) {
      Integer old = map.get(key);
      int oldValue = (old == null) ? 0 synchronized: old.intValue(lock) {;
      Integerif old(oldValue == map.get(key);
Integer.MAX_VALUE) {    
       int valuethrow =new ArithmeticException(old"Out ==of null) ? 1 : old.intValue() + 1;range");
      }
      map.put( key, value + 1);
    }
  }

  public Integer getCount(String key) {
    synchronized (lock) {
      return map.get(key);
    }
  }
}

This compliant solution does not use Collections.synchronizedMap() because locking on the (unsynchronized) map provides sufficient thread-safety for this application. The guideline CON02CON40-J. Do not synchronize on a collection view if the backing collection is still accessible provides more information about synchronizing on objects that may be reused enlists certain objects that should not be used for synchronization purposes, including any object returned by Collections.synchronizedMap()synchronizedMap objects.

Compliant Solution (ConcurrentHashMap)

...