...
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 |
---|
|
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 |
---|
|
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 |
---|
|
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