I am using persisting objects using JPA. The Main object has an owning One-Many relationship with another object. The other object is stored in a HashMap. What sort of synchronisation would fix this problem? It seems to happen at completely random times and is very unpredictable. Here is the exception I get:

Exception in thread "pool-1-thread-1" java.util.ConcurrentModificationException
        at java.util.HashMap$HashIterator.nextEntry(Unknown Source)
        at java.util.HashMap$ValueIterator.next(Unknown Source)
        at org.hibernate.collection.AbstractPersistentCollection$IteratorProxy.next(AbstractPersistentCollection.java:555)
        at org.hibernate.engine.Cascade.cascadeCollectionElements(Cascade.java:296)
        at org.hibernate.engine.Cascade.cascadeCollection(Cascade.java:242)
        at org.hibernate.engine.Cascade.cascadeAssociation(Cascade.java:219)
        at org.hibernate.engine.Cascade.cascadeProperty(Cascade.java:169)
        at org.hibernate.engine.Cascade.cascade(Cascade.java:130)
upvote
  flag
Can you provide some more context? Are you merging, updating or deleting an entity? What associations doest this entity have? What about your cascading settings? – ordnungswidrig
upvote
  flag
From the stack trace you can see that the Exception happens while iterating through the HashMap. Surely some other thread is modifying the map but the exception occurs in the thread that's iterating. – Chochos
upvote
  flag

5 Answers 11

It sounds less like a Java synchronization issue and more like a database locking problem.

I don't know if adding a version to all your persistent classes will sort it out, but that's one way that Hibernate can provide exclusive access to rows in a table.

Could be that isolation level needs to be higher. If you allow "dirty reads", maybe you need to bump up to serializable.

upvote
  flag
HashMap is thread-safe. Its not sunchronization issue. – TBH
1 upvote
  flag
Did you down vote me? Did you not read my answer? I said it was a database locking problem, not "synchronization". Your reading skills are as poor as your spelling. – duffymo
1 upvote
  flag
@TBH How is HashMap thread-safe? – Narayana Nagireddi
upvote
  flag
I think they meant Hashtable. It shipped as part of JDK 1.0. Like Vector, it was written to be thread safe - and slow. Both have been superseded by non-thread safe alternatives: HashMap and ArrayList. Pay for what you use. – duffymo
up vote 199 down vote accepted

This is not a synchronization problem. This will occur if the underlying collection that is being iterated over is modified by anything other than the Iterator itself.

Iterator it = map.entrySet().iterator();
while (it.hasNext())
{
   Entry item = it.next();
   map.remove(item.getKey());
}

This will throw a ConcurrentModificationException when the it.hasNext() is called the second time.

The correct approach would be

   Iterator it = map.entrySet().iterator();
   while (it.hasNext())
   {
      Entry item = it.next();
      it.remove();
   }

Assuming this iterator supports the remove() operation.

1 upvote
  flag
Possibly, but it looks as if Hibernate is doing the iterating, which should be implemented reasonably correctly. There could be callback modifying the map, but that is unlikely. The unpredictability points to an actual concurrency problem. – Tom Hawtin - tackline
upvote
  flag
This exception has nothing to do with threading concurrency, it is caused by the backing store of the iterator being modified. Whether by another thread of not doesn't matter to the iterator. IMHO it is a poorly named exception since it gives an incorrect impression of the cause. – Robin
upvote
  flag
I agree however that if it is unpredictable, there is most likely a threading issue which is causing the conditions for this exception to occur. Which makes it all the more confusing because of the exception name. – Robin
upvote
  flag
This is correct and a better explanation than the accepted answer, but the accepted answer is a nice fix. ConcurrentHashMap is not subject to CME, even inside an iterator (although the iterator is still designed for single-thread access). – Greg
upvote
  flag
This solution has no point, because Maps don't have iterator() method. Robin's example would be applicable to e.g. Lists. – peter
upvote
  flag
I think the point was still served, but you are correct in that my example was wrong. It has been fixed. – Robin
upvote
  flag
I was working on android and it took me like 4 hours to actually find something that pointed out my issue. Thanks alot. – Muneeb Mirza

Try either CopyOnWriteArrayList or CopyOnWriteArraySet depending on what you are trying to do.

Try using a ConcurrentHashMap instead of a plain HashMap

upvote
  flag
Did that really solve the problem? I am experiencing the same issue but I can most certainly rule out any threading issues. – tobiasbayer
2 upvote
  flag
Another solution is to create a copy of the map and iterate through that copy instead. Or copy the set of keys and iterate through them, getting the value for each key from the original map. – Chochos
upvote
  flag
It is Hibernate who is iterating through the collection so you cannot simply copy it. – tobiasbayer
upvote
  flag
Instant savior. Going to look into why this worked so well so I don't get more surprises further down the road. – Valchris
upvote
  flag
@CodeBrickie, yes, it somehow does solve the problem that @Robin wrote about. Anyway map = Collections.synchronizedMap(map); doesn't solve anything :) So this is not a threading issue, it appears that ConcurrentHashMap is more noob-safe :) – dantuch
upvote
  flag
ConcurrentHashMap can be modified while it is being iterated and it won't throw. The downside is slower performance. – Chochos
upvote
  flag
I guess its not synchronization issue it is problem if modification of same modification while looping same object. – Rais Alam
upvote
  flag
From ConcurrentHashMap Javadoc Similarly, Iterators, Spliterators and Enumerations return elements reflecting the state of the hash table at some point at or since the creation of the iterator/enumeration. They do not throw ConcurrentModificationException. However, iterators are designed to be used by only one thread at a time. – juanmf

Maybe another solution would be to acquire a lock before starting your modification/persistence in order not to have some other thread modify what your are iterating

private ReadWriteLock lock = new ReentrantReadWriteLock();
lock.writeLock().lock();
try{
//itterate and persist
}
finally{
lock.writeLock().unlock();
    }
  • If you are not doing any manipulation maybe lock.readLock().lock() is also OK

Not the answer you're looking for? Browse other questions tagged or ask your own question.