...
Note that the enhanced for
loop (for-each idiom) uses an Iterator
internally. Consequently, enhanced for
loops can also participate in concurrent modification issues, even though they lack an obvious iterator.
Noncompliant Code Example (Single-Threaded)
This noncompliant code example (based on a SDN 2008 bug report 6687277) uses the Collection
's remove()
method to remove an element from an ArrayList
while iterating over the ArrayList
. The resulting behavior is unspecified.
Code Block | ||
---|---|---|
| ||
class BadIterate { public static void main(String[] args) { List<String> list = new ArrayList<String>(); list.add("one"); list.add("two"); Iterator iter = list.iterator(); while(iter.hasNext()) { String s = (String)iter.next(); if(s.equals("one")) { list.remove(s); } } } } |
Compliant Solution (iterator.remove()
)
The Iterator.remove()
method removes from the underlying Collection
the last element returned by the iterator. Its behavior is fully specified, so it may be safely invoked while iterating over a collection.
Code Block | ||
---|---|---|
| ||
// ... if (s.equals("one")) { iter.remove(); } // ... |
Noncompliant Code Example (Multithreaded)
Although acceptable in a single-threaded environment, this noncompliant code example is insecure in a multithreaded environment because it is possible for another thread to modify the widgetList
while the current thread iterates over the widgetList
. Additionally, the doSomething()
method could also modify the collection during iteration.
Code Block | ||
---|---|---|
| ||
List<Widget> widgetList = new ArrayList<Widget>(); pubic void widgetOperation() { // May throw ConcurrentModificationException for (Widget w : widgetList) { doSomething(w); } } |
Compliant Solution (Thread-Safe Collection)
This compliant solution wraps the ArrayList
in a synchronized collection so that all modifications are subject to the locking mechanism.
...
Wiki Markup |
---|
This approach needs to be implemented correctly to avoid starvation, deadlock and scalability issues \[[Goetz 2006|AA. Bibliography#Goetz 06]\]. |
Compliant Solution (Deep Copying)
This compliant solution creates a deep copy of the mutable widgetList
before iterating over it.
...
Wiki Markup |
---|
Creating deep copies of the list prevents underlying changes in the original list from affecting the iteration in progress. "Since the clone is thread-confined, no other thread can modify it during iteration, eliminating the possibility of {{ConcurrentModificationException}}. (The collection still must be locked during the clone operation itself)" \[[Goetz 2006|AA. Bibliography#Goetz 06]\]. However, this approach is often more expensive than other techniques. There is also a risk of operating on stale data which may affect the correctness of the code. |
Compliant Solution (CopyOnWriteArrayList
)
The CopyOnWriteArrayList
data structure implements all mutating operations by making a fresh copy of the underlying array. It is fully thread-safe, and is optimized for cases where traversal operations vastly outnumber mutations. Note that traversals of such lists always see the list in the state it had at the creation of the iterator (or enhanced for loop); subsequent modifications of the list are invisible to an ongoing traversal. Consequently, this solution is inappropriate when mutations of the list are frequent or when new values should be reflected in ongoing traversals.
Code Block | ||
---|---|---|
| ||
List<Widget> widgetList = new CopyOnWriteArrayList<Widget>(); pubic void widgetOperation() { for (Widget w : widgetList) { doSomething(w); } } |
Exceptions
MSC08MSC06-EX0: The Iterator.remove()
method can be used to modify the underlying collection when an iteration is in progress.
Risk Assessment
Modifying a Collection while iterating over it can lead to nondeterministic behavior.
Rule | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
MSC08 MSC06-J | low | probable | medium | P4 | L3 |
Automated Detection
The Coverity Prevent Version 5.0 INVALIDATE_ITERATOR checker can detect the instance where an iterator is being used after the source container of the iterator is modified.
Related Vulnerabilities
The Apache Geronimo bug HARMONY-6236 documents an ArrayList
breaking when given concurrent collections as input.
Bibliography
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="0f72e9a5697a83d2-8066259d-418d4882-9e6ba53b-05f85d842c1fcd267cdc79ac"><ac:plain-text-body><![CDATA[ | [[API 2006 | AA. Bibliography#API 06]] | Class [ConcurrentModificationException | http://java.sun.com/j2se/1.5.0/docs/api/java/util/ConcurrentModificationException.html] | ]]></ac:plain-text-body></ac:structured-macro> |
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="2c8d98eefeaeb09e-0a7bdf28-4eab4885-8e228317-08f94a23040e69b64a944b55"><ac:plain-text-body><![CDATA[ | [[SDN 2008 | AA. Bibliography#SDN 08]] | [Sun Bug database, Bug ID:6687277 | http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6687277] | ]]></ac:plain-text-body></ac:structured-macro> |
<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="b21a76fc15590080-14b6e0f2-43c54954-956f94d7-6d18f0d79ee9aef9ba8c0898"><ac:plain-text-body><![CDATA[ | [[Goetz 2006 | AA. Bibliography#Goetz 06]] | 5.1.2. Iterators and Concurrentmodificationexception | ]]></ac:plain-text-body></ac:structured-macro> |
...