ConcurrentModificationException despite using synchronized

Question

 public synchronized X getAnotherX(){ 
  if(iterator.hasNext()){
   X b = iterator.next();
   String name = b.getInputFileName();
  ...
   return b;
  }
  else{return null;}
 }

despite the synchronized statement in the declaration header, i still get a ConcurrentModificationException Exception at the line where i use iterator.next(); whats wrong here ?

Answer 1

ConcurrentModificationException usually has nothing to do with multiple threads. Most of the time it occurs because you are modifying the collection over which it is iterating within the body of the iteration loop. For example, this will cause it:

Iterator iterator = collection.iterator();
while (iterator.hasNext()) {
    Item item = (Item) iterator.next();
    if (item.satisfiesCondition()) {
       collection.remove(item);
    }
}

In this case you must use the iterator.remove() method instead. This occurs equally if you are adding to the collection, in which case there is no general solution. However, the subtype ListIterator can be used if dealing with a list and this has an add() method.

Answer 2

I agree with the statements above about ConcurrentModificationException often happening as a result of modifying the collection in the same thread as iterating. However, it is not always the reason.

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.

Through bad luck (or Murphy's Law, "Anything that can go wrong, will go wrong."), these two threads can happen to execute at the same time. In the case of structural modifications of some widely-used collections (e.g. ArrayList, HashSet, HashMap etc), this can result in a ConcurrentModificationException.

It is hard to prevent the problem entirely:

• You can document synchronization requirements, e.g. inserting "you must synchronize on blah before modifying this collection" or "acquire bloo lock first", but that's relying upon users to discover, read, understand and apply the instruction.

  There is the javax.annotation.concurrent.GuardedBy annotation, which can help to document this in a standardized way; the problem is then that you have to have some means of checking correct use of the annotation in the toolchain. For example, you might be able to use something like Google's errorprone, which can check in some situations, but it's not perfect.

• For simple operations on collections, you can make use of the Collections.synchronizedXXX factory methods, which wrap a collection so that every method call synchronizes on the underlying collection first, e.g. the SynchronizedCollection.add method:

  @Override public boolean add(E e) {
    synchronized (mutex) { return c.add(obj); }
  }
  

  Where mutex is the synchronized-on instance (often the SynchronizedCollection itself), and c is the wrapped collection.

  The two caveats with this approach are:

  1. You have to be careful that the wrapped collection cannot be accessed in any other way, since that would allow non-synchronized access, the original problem. This is typically achieved by wrapping the collection immediately on construction:

     Collections.synchronizedList(new ArrayList<T>());
     

  2. The synchronization is applied per method call, so if you are doing some compound operation, e.g.

     if (c.size() > 5) { c.add(new Frob()); }
     

     then you don't have exclusive access throughout that operation, only for the size() and add(...) calls individually.

     In order to get mutually exclusive access for the duration of the compound operation, you would need to externally synchronize, e.g. synchronized (c) { ... }. This requires you to know the correct thing to synchronize on, however, which may or may not be c.

Answer 3

Below example is just the demo for this:

public class SynchronizedListDemo {
public static void main(String[] args) throws InterruptedException {
    List<Integer> list = new ArrayList<>();
    for(int i=0;i<100000;i++){
        System.out.println(i);
        list.add(i);
    }
    // Synchronzied list will also give ConcurrentModificationException, b'coz
    // it makes only methods thread safe, but you are still modifying list while iterating it.
    // You can use 'ListIterator' or 'CopyOnWriteArrayList'
    List<Integer> list1 = Collections.synchronizedList(list);

    Runnable r1= ()->{
        for(Integer i: list1)
            System.out.println(i);
    };
    Runnable r2 = ()->{
        try {
            System.out.println();
            System.out.println("Removing....");
            //list1.add(4); // Will give ConcurrentModificationException
            System.out.println("Removed");
        } catch (Exception e) {
            e.printStackTrace();
        }
    };

    // This will not give ConcurrentModificationException as it work on the copy of list.
    List<Integer> list2 = new CopyOnWriteArrayList<>(list);

    Runnable r3= ()->{
        for(Integer i: list2)
            System.out.println(i);
    };
    Runnable r4 = ()->{
        try {
            System.out.println();
            System.out.println("Removing....");
            list2.add(4);
            System.out.println("Removed");
        } catch (Exception e) {
            e.printStackTrace();
        }
    };

    Thread t1 = new Thread(r3);
    Thread.sleep(100);
    Thread t2 = new Thread(r4);
    t1.start();
    t2.start();

    System.out.println("Done");
}

}