For loop in syntax-highlighted code on a dark screen, illustrating a listener loop that throws ConcurrentModificationException

Why a Listener That Removes Itself Throws ConcurrentModificationException

by

A listener that unregisters itself inside its callback changes the list while a for loop still walks it. Kotlin’s for loop runs on the list’s iterator. The iterator of ArrayList is fail-fast. It notices the structural change and throws ConcurrentModificationException on the next call to next(). No second thread is involved. A second cause makes the bug hard to pin down. Whether the loop crashes depends on where the listener sits in the list. It also depends on the runtime, because the JVM and Android decide hasNext() differently. In one position, each runtime ends the loop early with no crash. The remaining listener never hears the event. So a JVM unit test can pass while the phone crashes. Copying the list stops the crash. But the copy still calls a listener that an earlier listener just removed. The fix is to iterate a snapshot and check each listener is still registered before calling it.

A sync tracker that crashes on one thread

The running example is a SyncTracker. It tells every registered screen when a background sync ends. One caller wants the event only once, so its listener unregisters itself on the first call. Everything runs on the main thread.

fun interface SyncListener { fun onSyncFinished(ok: Boolean) }

class SyncTracker {
    private val listeners = mutableListOf<SyncListener>()

    fun register(l: SyncListener) { listeners += l }
    fun unregister(l: SyncListener) { listeners -= l }

    fun notifyFinished(ok: Boolean) {
        for (l in listeners) l.onSyncFinished(ok)
    }
}

// A one-shot caller: show a snackbar once, then stop listening
tracker.register(object : SyncListener {
    override fun onSyncFinished(ok: Boolean) {
        showSnackbar(ok)
        tracker.unregister(this)
    }
})

Here are four registration orders run through this class on the JVM, with Kotlin 2.4.20. The backing list is a java.util.ArrayList. Each line shows which listeners were called before the loop stopped.

toast, oneShot, badge   -> no crash, called=[toast, oneShot]
a, b, oneShot, badge    -> no crash, called=[a, b, oneShot]
oneShot, toast, badge   -> ConcurrentModificationException, called=[oneShot]
toast, badge, oneShot   -> ConcurrentModificationException, called=[toast, badge, oneShot]

Two of the four orders crash. In the other two, nothing is thrown. But badge never gets the event. That silent case is worse than the crash, because nothing in the logs points at it.

Why ConcurrentModificationException fires on a single thread

The Kotlin control-flow docs say the for loop “iterates through anything that provides an iterator.” A collection provides one by default. So for (l in listeners) calls listeners.iterator(), then hasNext() and next() on each pass. listeners.forEach { } is an inline function over the same iterator, so it fails the same way.

The Java 21 ArrayList reference calls that iterator fail-fast. A structural change made outside the iterator’s own remove or add makes it throw ConcurrentModificationException. Removing an element counts as structural. The list tracks this with a counter called modCount. The iterator copies the counter when it’s created. In the OpenJDK source, next() compares the two before returning anything.

Here “concurrent” means during iteration. No other thread is involved. The one-shot listener calls unregister(this) from inside the loop body. That bumps modCount. The next call to next() sees the mismatch and throws.

hasNext() decides whether you crash or silently skip

The check lives in next(), not in hasNext(). If hasNext() returns false first, the loop ends quietly. That’s the second cause. It differs by runtime.

On the JVM, OpenJDK’s hasNext() returns cursor != size. It reads the live size. Say the one-shot listener is third of four. After it runs, the cursor is 3 and the size has dropped to 3. hasNext() says false. The loop exits. The fourth listener is skipped without an error.

Android doesn’t run OpenJDK’s class unchanged. AOSP’s libcore copy of ArrayList marks an “Android-changed” limit field. It stores the size at the moment the iterator is created. hasNext() returns cursor < limit. The AOSP comment says this keeps hasNext() stable while elements are added or removed. The side effect flips which position is silent.

The model below copies both hasNext() rules. It keeps the same modCount check in next(). It runs as plain Kotlin on the JVM. So the Android column comes from the AOSP source above, not from ART.

class ModelList<T>(private val androidRule: Boolean) : Iterable<T> {
    private val items = ArrayList<T>()
    private var modCount = 0
    fun add(x: T) { items.add(x); modCount++ }
    fun remove(x: T) { if (items.remove(x)) modCount++ }

    override fun iterator() = object : Iterator<T> {
        val limit = items.size // Android: size captured at creation
        var cursor = 0
        val expected = modCount
        override fun hasNext() =
            if (androidRule) cursor < limit else cursor != items.size
        override fun next(): T {
            if (modCount != expected) throw ConcurrentModificationException()
            return items[cursor++]
        }
    }
}
oneShot at index 0 of 4 | JVM: CME                          | Android: CME
oneShot at index 1 of 4 | JVM: CME                          | Android: CME
oneShot at index 2 of 4 | JVM: no crash, L3 never called    | Android: CME
oneShot at index 3 of 4 | JVM: CME                          | Android: no crash

The JVM rows match the real ArrayList run from the first section. That matters for tests. Local unit tests run on your workstation’s JVM, not on a device. A test that registers the one-shot listener third of four doesn’t crash there. The same order crashes on the phone. Put it last and the reverse happens.

The Java docs warn against leaning on any of this. They say the exception is thrown “on a best-effort basis.” Code that depends on it for correctness is wrong.

Copying the list stops the crash but calls a removed listener

The usual first fix is to iterate a copy: listeners.toList().forEach { }. Another is to swap in CopyOnWriteArrayList. Both stop the crash. Both also create a new bug once one listener removes a different one.

Extend the example. A navigation listener reacts to the sync by closing a detail screen. Closing the screen unregisters that screen’s listener. The screen’s listener sits after the navigation listener in the list.

class DetailScreen(private val log: MutableList<String>) {
    var destroyed = false
    val listener = SyncListener {
        log += if (destroyed) "screen(DESTROYED)" else "screen"
    }
}

tracker.register(oneShot)                  // unregisters itself
tracker.register {                         // navigation
    screen.destroyed = true
    tracker.unregister(screen.listener)
}
tracker.register(screen.listener)
tracker.register(badge)
toList snapshot      -> [oneShot, nav, screen(DESTROYED), badge]
CopyOnWriteArrayList -> [oneShot, nav, screen(DESTROYED), badge]

The destroyed screen still gets the event. On a device, that could be a callback touching a view after onDestroyView(). The CopyOnWriteArrayList reference documents why. Its iterator reads a snapshot of the array taken when the iterator was created. It “will not reflect additions, removals, or changes” made after that. toList() behaves the same way, because it’s a copy too.

The fix: iterate a snapshot, then recheck membership

@MainThread
fun notifyFinished(ok: Boolean) {
    for (l in listeners.toList()) {
        // Skip anyone unregistered earlier in this same dispatch
        if (l in listeners) l.onSyncFinished(ok)
    }
}
snapshot + recheck   -> [oneShot, nav, badge]

The snapshot makes the loop immune to changes in listeners. The membership check keeps removals in effect for the rest of the dispatch. A listener added during the dispatch waits for the next event, because it isn’t in the snapshot. That’s usually what you want. Say it out loud in an interview anyway.

The in check is a linear scan. For a handful of listeners, that cost doesn’t matter. If the list is large, back it with a LinkedHashSet to keep order and get fast lookups.

This version assumes one thread, so the @MainThread annotation documents the contract. If other threads can register, use CopyOnWriteArrayList for the storage. Keep the same membership check in the loop.

This is also what LiveData does. LiveData.java stores observers in a SafeIterableMap. Its class comment describes a linked list that “supports modifications during iterations.” Removing an entry updates any live iterator. removeObserver() also marks the wrapper inactive. considerNotify() checks that flag first. So an observer removed mid-dispatch never gets the value.

What to say when the interviewer hands you this snippet

  1. Name the mechanism first. The for loop uses the list’s iterator. unregister(this) changes the list inside the loop body, so the fail-fast check in next() throws. It’s a single-thread bug.
  2. Explain why it’s intermittent. The check runs only in next(). If hasNext() says false first, the loop ends silently and later listeners are skipped. Mention that JVM and Android pick different positions for that silent exit.
  3. Propose the copy, then break it yourself. A snapshot or CopyOnWriteArrayList stops the crash. It still calls a listener that an earlier listener removed.
  4. Give the final version. Iterate a snapshot and recheck membership before each call. Then state the threading contract. Point to LiveData as the same design in androidx.

This fits the spot-the-bug format covered in the Android live-coding round formats. Step 3 goes past finding the crash, because you break your own first fix. The debugging round scores that follow-up bug too. See what the debugging round scores after the first fix.

Answers that sound right and still fail

  • “It’s a threading bug, so add synchronized.” There’s one thread. A JVM lock is reentrant, so the same thread walks straight back in. A nested synchronized version still throws ConcurrentModificationException.
  • “Use iterator.remove().” That works when the loop does the removing. Here the listener removes itself through unregister(). It never sees the iterator.
  • “Loop by index instead.” for (i in 0 until listeners.size) computes the range once. After the one-shot listener leaves, every later index shifts down by one. In the four-listener example, the navigation listener was skipped. The loop then threw IndexOutOfBoundsException at the end.
  • “Switch to CopyOnWriteArrayList and you’re done.” It stops the crash. It still delivers to listeners removed during the same dispatch.
  • “Catch ConcurrentModificationException.” The Java docs say the check is best-effort and only for detecting bugs. The catch also hides the listeners that were never called.

A related bug explains why fast scrolling duplicates items in a paginated list. That bug hands out the live list. A caller iterating it hits the same exception.

Leave a Comment