Java - ConcurrentModificationException

Question

The following piece of code throws a ConcurrentModificationException almost every time it is called. The second piece of code does not throw the exception, however it is not the correct logic that I need. If the object is an instance of EditorFrame, I need to invoke a custom disposal strategy which is what the close() method is. However if it is just a basic frame I want it to call dispose().

I've looked around this site and followed some instructions, but none that I found have worked.

The code that throws the exception:

synchronized (frameList) {
    for (Iterator<JFrame> it = frameList.iterator(); it.hasNext();) {
        JFrame frame = it.next();
        if (frame instanceof EditorFrame) ((EditorFrame) frame).close();
        else frame.dispose();
        it.remove();
    }
}

This code works, but it's not what I want:

synchronized (frameList) {
    for (Iterator<JFrame> it = frameList.iterator(); it.hasNext();) {
        JFrame frame = it.next();
        frame.dispose();
        it.remove();
    }
}

Thanks for helping!

Does you `EditorFrame.close` method modify `frameList`? (The current thread will hold the lock on `frameList`. Locks are re-entrant. Shouldn't need a lock, as Swing should be used only from the Event Dispatch Thread (EDT).) — Tom Hawtin - tackline, Jan 27 '12 at 00:19
Agree with Tom Hawtin: you should only be doing this code on one single thread, the EDT, and so should not need synchronization. Which line throws the exception by the way? Also, all if/else blocks, for loops, shoot any code block should be enclosed in curly braces to avoid ambiguity. — Hovercraft Full Of Eels, Jan 27 '12 at 00:43
does frame.close() remove frame from frameList, or modify it in any way? that would explain the ConcurrentModificationException even from a single thread. — Nitzan Volman, Jan 27 '12 at 01:21

score 6 · Accepted Answer · answered Jan 27 '12 at 01:26

Without getting in to exactly what is causing the ConcurrentModificationException. you are still removing every object from frameList

why don't you clear the list explicitly after you finished iterating the list.

synchronized (frameList) {
    for (JFrame frame : frameList) {
        if (frame instanceof EditorFrame) ((EditorFrame) frame).close();
        else frame.dispose();
    }
    frameList.clear();
}

Java - ConcurrentModificationException

1 Answers1