CODEWITHSUNDEEP
javaloopsarraylistconcurrencyiterator
AWT
AWT

Reputation: 3707

ArrayList iterator throwing ConcurrentModificationException

I have an ArrayList with two accessor methods and a notifier. My list:

private final List<WeakReference<LockListener>> listeners = new ArrayList<>();

All subscribe operations use this:

public void subscribe(@NonNull LockListener listener) {
    for (Iterator<WeakReference<LockListener>> it = listeners.iterator(); it.hasNext(); ) {
        // has this one already subscribed?
        if (listener.equals(it.next().get())) {
            return;
        }
    }
    listeners.add(new WeakReference<>(listener));
}

All unsubscribe operations use this:

public void unsubscribe(@NonNull LockListener listener) {
    if (listeners.isEmpty()) {
        return;
    }

    for (Iterator<WeakReference<LockListener>> it = listeners.iterator(); it.hasNext(); ) {
        WeakReference<LockListener> ref = it.next();
        if (ref == null || ref.get() == null || listener.equals(ref.get())) {
            it.remove();
        }
    }
}

And the notifier:

private void notifyListeners() {
    if (listeners.isEmpty()) {
        return;
    }

    Iterator<WeakReference<LockListener>> it = listeners.iterator();
    while (it.hasNext()) {
        WeakReference<LockListener> ref = it.next();
        if (ref == null || ref.get() == null) {
            it.remove();
        } else {
            ref.get().onLocked();
        }
    }
}

What I'm seeing in my testing is that it.next() in notifyListeners() occasionally throws a ConcurrentModificationException. My guess is that this is due to listeners.add() in the subscriber method.

I guess I had a misunderstanding of the iterator here. I was under the assumption that iterating over the list protected me from concurrency issues caused by add/remove operations.

Apparently I'm wrong here. Is it that the iterator is only a protection from ConcurrentModificationException while changing the collection you're iterating? For example, calling remove() on your list while iterating would throw an error, but calling it.remove() is safe.

In my case, subscribing calls add() on the same list as it is being iterated. Is my understanding here correct?

Upvotes: 2

Views: 166

Answers (1)

Rolf Sch&#228;uble
Rolf Sch&#228;uble

Reputation: 690

If I read your last sentence correctly, the three methods in your example are called concurrently from several threads. If this is indeed the case, then this is your problem.

ArrayList is not thread-safe. Modifying it concurrently without additional synchronization results in undefined behavior, no matter if you modify it directly or using an iterator.

You could either synchronize access to the list (e.g. making the three methods synchronized), or use a thread-safe collection class like ConcurrentLinkedDeque. In case of the latter, make sure to read the JavaDoc (especially the part about iterators being weekly consistent) to understand what is guaranteed and what is not.

Upvotes: 2

Related Questions