...
Given an invariant involving multiple objects, a programmer may might incorrectly assume that a group of individually atomic operations is collectively atomic without additional locking. Similarly, programmers may might incorrectly assume that use of a thread-safe Collection is sufficient to preserve an invariant that involves the collection's elements without additional synchronization. A thread-safe class can only guarantee atomicity of its individual methods. A grouping of calls to such methods requires additional synchronization.
...
Compound operations on shared variables are also non-atomic. For more information, see See rule VNA02-J. Ensure that compound operations on shared variables are atomic for more information.
Rule VNA04-J. Ensure that calls to chained methods are atomic describes a specialized case of this rule.
...
| Code Block | ||
|---|---|---|
| ||
final class IPHolder {
private final List<InetAddress> ips =
Collections.synchronizedList(new ArrayList<InetAddress>());
public void addAndPrintIPAddresses(InetAddress address) {
ips.add(address);
InetAddress[] addressCopy =
(InetAddress[]) ips.toArray(new InetAddress[0]);
// Iterate through array addressCopy ...
}
}
|
Individually, the add() and toArray() collection methods are atomic. However, when called in succession (for example, in the addAndPrintIPAddresses() method), they lack any there is no guarantee that the combined operation is atomic. The addAndPrintIPAddresses() method contains a race condition that allows one thread to add to the list and a second thread to race in and modify the list before the first thread completes. Consequently, the addressCopy array may contain more IP addresses than expected.
...
| Code Block | ||
|---|---|---|
| ||
final class IPHolder {
private final List<InetAddress> ips =
Collections.synchronizedList(new ArrayList<InetAddress>());
public void addAndPrintIPAddresses(InetAddress address) {
synchronized (ips) {
ips.add(address);
InetAddress[] addressCopy =
(InetAddress[]) ips.toArray(new InetAddress[0]);
// Iterate through array addressCopy ...
}
}
}
|
...
This code does not violate rule LCK04-J. Do not synchronize on a collection view if the backing collection is accessible because, while it does synchronize on a collection view (the synchronizedList result), the backing collection is inaccessible and consequently cannot be modified by any code.
...
| Wiki Markup |
|---|
This noncompliant code example defines the {{KeyedCounter}} class that is not thread-safe. Although the {{HashMap}} is wrapped in a {{synchronizedMap()}}, the overall increment operation is fails tonot be atomic \[[Lee 2009|AA. Bibliography#Lee 09]\]. |
| Code Block | ||
|---|---|---|
| ||
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 oldValue = (old == null) ? 0 : old.intValue();
if (oldValue == Integer.MAX_VALUE) {
throw new ArithmeticException("Out of range");
}
map.put( key, oldValue + 1);
}
public Integer getCount(String key) {
return map.get(key);
}
}
|
...
| Code Block | ||
|---|---|---|
| ||
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 : old.intValue();
if (oldValue == Integer.MAX_VALUE) {
throw new ArithmeticException("Out of range");
}
map.put(key, oldValue + 1);
}
}
public Integer getCount(String key) {
synchronized (lock) {
return map.get(key);
}
}
}
|
...
The previous compliant solution is safe for multithreaded use , but fails to does not scale well because of excessive synchronization, which can lead to contention and deadlock.
...
| Code Block | ||
|---|---|---|
| ||
final class KeyedCounter {
private final ConcurrentMap<String, AtomicInteger> map =
new ConcurrentHashMap<String, AtomicInteger>();
public void increment(String key) {
AtomicInteger value = new AtomicInteger();
AtomicInteger old = map.putIfAbsent(key, value);
if (old != null) {
value = old;
}
if (value.get() == Integer.MAX_VALUE) {
throw new ArithmeticException("Out of range");
}
value.incrementAndGet(); // Increment the value atomically
}
public Integer getCount(String key) {
AtomicInteger value = map.get(key);
return (value == null) ? null : value.get();
}
// Other accessors ...
}
|
...
Related Guidelines
CWE-362, ". Concurrent Execution execution using Shared Resource with Improper Synchronization ('Race Condition')" shared resource with improper synchronization ("race condition") | |
| CWE-366, ". Race Condition condition within a Thread" thread |
| CWE-662, ". Improper Synchronization" synchronization |
Bibliography
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="1c4fd592fadba82b-d51e66f4-4f37492d-b3478672-6ee53416a96cd48070305d94"><ac:plain-text-body><![CDATA[ | [[API 2006 | AA. Bibliography#API 06]] |
| ]]></ac:plain-text-body></ac:structured-macro> |
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="706984ff69021500-4b019c96-4f9b4a38-89269b4e-dd82accee3fd7a3f26d72608"><ac:plain-text-body><![CDATA[ | [[Goetz 2006 | AA. Bibliography#Goetz 06]] | Section 4.4.1 ", Client-side Locking " | ]]></ac:plain-text-body></ac:structured-macro> |
| Section 5.2.1, " ConcurrentHashMap " | |||
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="2c1a3d62d462a92c-0fd10b12-434e46cf-a4c68339-73b9c41ebc0201e72e23e322"><ac:plain-text-body><![CDATA[ | [[JavaThreads 2004 | AA. Bibliography#JavaThreads 04]] | Section 8.2, " Synchronization and Collection Classes " | ]]></ac:plain-text-body></ac:structured-macro> |
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="265dbd4f1f59ef71-a30a9854-48444d5b-b0e3995f-caf2cb031ae6cffc6029c708"><ac:plain-text-body><![CDATA[ | [[Lee 2009 | AA. Bibliography#Lee 09]] | " Map & Compound Operation " | ]]></ac:plain-text-body></ac:structured-macro> |
...