Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: added NCEs/CS

...

Extension is more fragile than adding code directly to a class, because the implementation of the synchronization policy is now distributed over multiple, separately maintained source files. If the underlying class were to change its synchronization policy by choosing a different lock to guard its state variables, the subclass would subtly and silently break, because it no longer used the right lock to control concurrent access to the base class's state.

Noncompliant Code Example

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

Code Block
bgColor#ccccff

public class KeyedCounter {
  private Map<String,Integer> map =
    Collections.synchronizedMap(new HashMap<String,Integer>());

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

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

Compliant Solution

Wiki Markup
This compliant solution declares the {{increment()}} method as {{synchronized}} to ensure atomicity. \[[Lee 09|AA. Java References#Lee 09]\] 

Code Block
bgColor#ccccff

public class KeyedCounter {
  private Map<String,Integer> map = new HashMap<String,Integer>();
 
  public synchronized void increment(String key) {
    Integer old = map.get(key);
    int value = (old == null) ? 1 : old.intValue()+1;
    map.put(key, value);
  }

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

Compliant Solution

Wiki Markup
The previous compliant solution does not scale very well. The class {{ConcurrentHashMap}} provides several utility methods to perform atomic operations and is used in this compliant solution.\[[Lee 09|AA. Java References#Lee 09]\] 

Code Block
bgColor#ccccff

public class KeyedCounter {
  private final ConcurrentMap<String, AtomicInteger> map =
    new ConcurrentHashMap<String, AtomicInteger>();

  public void increment(String key) {
    AtomicInteger value = new AtomicInteger(0);
    AtomicInteger old = map.putIfAbsent(key, value);
   
    if (old != null) { 
      value = old; 
    }

    value.incrementAndGet(); // increment the value atomically
  }

  public Integer getCount(String key) {
    AtomicInteger value = map.get(key);
    return (value == null) ? null : value.get();
  }
}

Risk Assessment

Non-atomic code can induce race conditions and affect program correctness.

...

Wiki Markup
\[[API 06|AA. Java References#API 06]\] Class Vector, Class WeakReference
\[[JavaThreads 04|AA. Java References#JavaThreads 04]\] 8.2 "Synchronization and Collection Classes"
\[[Goetz 06|AA. Java References#Goetz 06]\] 4.4.1. Client-side Locking and 4.4.2. Composition
\[[Lee 09|AA. Java References#Lee 09]\] "Map & Compound Operation"

...

CON37-J. Never apply a lock to methods making network calls      10. Concurrency (CON)      CON39-J. Ensure atomicity of 64-bit operations