Java - ConcurrentModificationException in multithreaded program, Set is wrapped with synchronized

Question

I have a crash in the following code:

private LocalBundle(Bundle b, Map<Bundle, LocalBundle> clonnedObjs) {
    this();
    synchronized (b.getVariables()) { // b.getVariables() is Set
        addFrom(b, clonnedObjs);
    }
}

but in addFrom I get the crash:

private synchronized void addFrom(Bundle b, Map<Bundle, LocalBundle> clonnedObjs) {
    Set<TaintedVariable> variablesOfBundle = b.getVariables();
    for(TaintedVariable v : variablesOfBundle) {

Exception message:

Exception in thread "pool-4-thread-1" java.util.ConcurrentModificationException
    at java.util.HashMap$HashIterator.nextNode(Unknown Source)
    at java.util.HashMap$KeyIterator.next(Unknown Source)
    at x.y.z.LocalBundle.addFrom(VisitedNodesWithBundle.java:198)

Does someone know why it happens? I wrapped with synchronized (b.getVariables()), but it looks like that two threads are executing for(TaintedVariable v : variablesOfBundle)

"it looks like that two threads are executing" Not necessarily. You can get a `ConcurrentModificationException` with a single thread. — Andy Turner, Jul 05 '16 at 08:17
Possible duplicate of [ConcurrentModificationException despite using synchronized](http://stackoverflow.com/questions/1655362/concurrentmodificationexception-despite-using-synchronized) — Raedwald, Jul 05 '16 at 08:45

score 3 · Accepted Answer · answered Jul 05 '16 at 08:39

The thing to remember about synchronized is that it only guarantees exclusive access if everybody accessing the shared resource also synchronizes.

For example, you can synchronize access to a shared variable:

synchronized (foo) {
  foo.setBar();
}

And you can think that you have exclusive access to it. However, there is nothing to stop another thread just doing something without the synchronized block:

foo.setBar();  // No synchronization first.

From your description, it sounds like this is happening; you've not given any details of what the "rogue" thread is doing, though.

If it is simply adding elements to the set, you can make your set synchronized when you create it:

Collections.synchronizedSet(new HashSet<TaintedVariable>());

Now individual calls to methods on the set will be synchronized, and so you would no longer get a CME. Note, however, that the synchronization is applied per method call: if you make a sequence of calls (e.g. if set contains foo then add bar), you need an explicit synchronized block around that logic.

score 2 · Answer 2 · answered Jul 05 '16 at 08:20

I'm not sure, that the reason is another thread. Take a closer look at javadoc for this exception. It's said that:

Note that this exception does not always indicate that an object has been concurrently modified by a different thread. If a single thread issues a sequence of method invocations that violates the contract of an object, the object may throw this exception. For example, if a thread modifies a collection directly while it is iterating over the collection with a fail-fast iterator, the iterator will throw this exception.

That means, that you may cause such exception in a single thread if you try to modify the collection while you are iterating over it.

You have to check out, whether your code tries to modify the variablesOfBundle collection in this for-loop

for(TaintedVariable v : variablesOfBundle)

It's not necessarily in that loop; that's just where I'd suspect it is. For instance, there could be another thread in the code which modifies `b.getVariables()` without first attempting to synchronize on it. — Andy Turner, Jul 05 '16 at 08:23
@AndyTurner, looks like you're right. It's modified in another thread, but by another code in another class (since inside the loop is no any modification of `variablesOfBundle`) — Ivan, Jul 05 '16 at 08:27

Java - ConcurrentModificationException in multithreaded program, Set is wrapped with synchronized

2 Answers2