Understanding and resolving ConcurrentModificationException

Question

I'm dealing with an intermittent, hard to reproduce ConcurrentModificationException in a chunk of legacy code:

class LegacyCode {
    private final WeakHashMap<A, B> mItems = new WeakHashMap<A, B>();

    public void someWork() {
        final List<A> copy = new LinkedList<A>();

        for (final A item : mItems.keySet()) {
            copy.add(item);
        }

        for (final A item : copy) {
            item.someMethod();
        }
    }

    public void addItem(final A item) {
        mItems.put(item, new B());
    }

    public void removeItem(final A item) {
        mItems.remove(item);
    }
}

The CME is being thrown in:

for (final A item : mItems.keySet()) {
    copy.add(item);
}

I'm not entirely sure as to why we create copy in this manner. The CME is thrown because either addItem(A) or removeItem(A) is called while the for-each loop is running.

Questions

1. Is my understanding of why CME is being thrown correct?

2. Will I avoid CME if I replace the for-each loop with:

   final List<A> copy = new LinkedList<A>(mItems.keySet());

3. Will this change be equivalent to the for-each loop we will be replacing? As far as I can tell, both snippets create a shallow copy of mItems.keySet() in copy.

Answer 1

Is my understanding of why CME is being thrown correct?

Absolutely. This is exactly what is happening.

Will I avoid CME if I replace the for-each loop with:

final List<A> copy = new LinkedList<A>(mItems.keySet());

No, you would not, because constructor of LinkedList<A> would have a similar loop. So you are correct in saying that this change will be equivalent to the for-each loop we will be replacing.

As far as fixing this problem is concerned, there is no off-the-shelf concurrent replacement for WeakHashMap class in Java standard library. You can address this the obvious way by making addItem and removeItem synchronized, and adding a synchronized block around the loop constructing a copy. You can also look into third-party collections that solve this problem without using synchronized in your code.