ArrayList iterator throwing ConcurrentModificationException

Question

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?

If you read the docs for iterator it will tell you that you can't modify the underlying structure — bichito
– bichito, Commented May 18, 2017 at 16:14

Rolf Schäuble · Accepted Answer · 2017-05-23 12:16:45Z

2

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.

answered May 23, 2017 at 12:16

Rolf Schäuble

6904 silver badges15 bronze badges

Sign up to request clarification or add additional context in comments.

Collectives™ on Stack Overflow

ArrayList iterator throwing ConcurrentModificationException

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related