Concurrent Modification Exception

java collections concurrency

15,240

Solution 1

If you constructed LinkedHashMap with accessOrder = true then LinkedHashMap.get() actually mutates the LinkedHashMap since it stores the most recently accessed entry at the front of the linked list of entries. Perhaps something is calling get() while the array list is making its copy with the Iterator.

Solution 2

This exception does not normally have anything to do with synchronization - it is normally thrown if a a Collection is modified while an Iterator is iterating through it. AddAll methods may use an iterator - and its worth noting that the posh foreach loop iterates over instances of Iterator too.

e.g:

for(Object o : objects) {
    objects.remove(o);
}

is sufficient to get the exception on some collections (e.g ArrayList).

James

Solution 3

Are those all function you have in your wrapper? Because this exception could be thrown while you somehow iterating over collection in another place. And I guess you synchronized method with potential obvious race condition, but probably have missed less obvious cases. Here the reference to exception class docs.

Solution 4

From the Javadoc:

If multiple threads access a linked hash map concurrently, and at least one of the threads modifies the map structurally, it must be synchronized externally. This is typically accomplished by synchronizing on some object that naturally encapsulates the map. If no such object exists, the map should be "wrapped" using the Collections.synchronizedMap method. This is best done at creation time, to prevent accidental unsynchronized access to the map:

Map m = Collections.synchronizedMap(new LinkedHashMap(...));

It may be safer for you to actually wrap the LinkedHashMap rather than claim you extend it. So your implementation would have an internal data member which is the Map returned by Collections.synchronizedMap(new LinkedHashMap(...)).

Please see the Collections javadoc for details: Collections.synchronizedMap

View more solutions

15,240

Author by

Cambium

Updated on July 09, 2022

Comments

Cambium almost 2 years
I'm currently working on a multi-threaded application, and I occasionally receive a concurrently modification exception (approximately once or twice an hour on average, but occurring at seemingly random intervals).

The faulty class is essentially a wrapper for a map -- which extends LinkedHashMap (with accessOrder set to true). The class has a few methods:
```
synchronized set(SomeKey key, SomeValue val)
```
The set method adds a key/value pair to the internal map, and is protected by the synchronized keyword.
```
synchronized get(SomeKey key)
```
The get method returns the value based on the input key.
```
rebuild()
```
The internal map is rebuilt once in a while (~every 2 minutes, intervals do not match up with the exceptions). The rebuild method essentially rebuilds the values based on their keys. Since rebuild() is fairly expensive, I did not put a synchronized keyword on the method. Instead, I am doing:
```
public void rebuild(){
  /* initialization stuff */
  List<SomeKey> keysCopy = new ArrayList<SomeKey>();
  synchronized (this) {
    keysCopy.addAll(internalMap.keySet());
  }
  /* 
    do stuff with keysCopy, update a temporary map
   */    
  synchronized (this) {
    internalMap.putAll(tempMap);
  }
}
```
The exception occurs at
```
keysCopy.addAll(internalMap.keySet());
```
Inside the synchronized block.

Suggestions are greatly appreciated. Feel free to point me to specific pages/chapters in Effective Java and/or Concurrency in Practice.

Update 1:

Sanitized stacktrace:
```
java.util.ConcurrentModificationException
        at java.util.LinkedHashMap$LinkedHashIterator.nextEntry(LinkedHashMap.java:365)
        at java.util.LinkedHashMap$KeyIterator.next(LinkedHashMap.java:376)
        at java.util.AbstractCollection.toArray(AbstractCollection.java:126)
        at java.util.ArrayList.addAll(ArrayList.java:473)
        at a.b.c.etc.SomeWrapper.rebuild(SomeWraper.java:109)
        at a.b.c.etc.SomeCaller.updateCache(SomeCaller.java:421)
        ...
```
Update 2:

Thanks everyone for the answers so far. I think the problem lies within the LinkedHashMap and its accessOrder attribute, although I am not entirely certain atm (investigating).

If accessOrder on a LinkedHashMap is set to true, and I access its keySet then proceed to add the keySet to a linkedList via addAll, do either of these actions mutate the order (i.e. count towards an "access")?
Cambium almost 15 years

Thanks for the suggestion, can you elaborate a little as to why that might solve the problem? I will give it a try shortly.
Cambium almost 15 years

Yes, all of those methods are in the wrapper class.
Cambium almost 15 years

No, keySet() is not overriden. I don't think this will cause a problem though because the only way to access this internal map is through the wrapper class's set(), get() and rebuild().
Skip Head almost 15 years

Is there any code in your project that uses an iterator for this class? Are those uses synchronized properly?
Cambium almost 15 years

This class has no iterator. The internal map can only be modified via the three methods I listed above.
Cambium almost 15 years

Hi James, I fail to see how this is relevant. I don't think I am modifying the map/keyset of the map while iterating through it.
Cambium almost 15 years

I completely agree with your analysis, however, I triple/quadruple checked my class, and made sure that those three methods are the only accessors of the internal map; and of course, internal map is private.
Cambium almost 15 years

Assuming no one calls get() while it's doing the copy, could this still trigger the exception? You raise a very good point though, I will look into the LinkedHashMap implementations. Thanks!
Michael Brewer-Davis almost 15 years

Based on your comment above that you use accessOrder, and assuming you have to call get() within the rebuild function (since all you show is grabbing keys), this seems a likely culprit. Do the rebuild() calls ever overlap?
Sean McCauliff almost 15 years

I imagine methods like containsKey() or containsValue() would also mutate the map, but the documentation for LinkedHashMap is silent on this issue.
Reed almost 15 years

damn, I meant to say volatile, as Jack mentions below
mP. almost 15 years

Given that the internal map can only be accessed via the wrapper changing the lock will make no difference. You will just be moving text around for no benefit.
gub almost 15 years

It is relevant because the exception is being thrown from your code almost certainly in the manner I describe.