java.util.ConcurrentModificationException - Android

Question

I am getting the following exception from the following piece of code sometimes People suggest that if I make the for loop to an iterator, It would fix the issue. But I don't understand If I am not removing any element inside the for loop but calling pendingActions.clear(); later. Please let me know if anyone has come across any of this scenario. Also , Would you think if I make list declaration to

private List<Action> pendingActions = Collections.synchronizedList(new ArrayList<>());

will fix the issue ?

 private List<Action> pendingActions = new ArrayList<>();

private void runPendingNavigationActions() {
        if (!pendingActions.isEmpty()) {
            for (Action r : pendingActions) {
                r.run();
            }
            pendingActions.clear();
        }
    }

Exception:

at android.app.ActivityThread.performResumeActivity(ActivityThread.java:4156)
    at android.app.ActivityThread.handleResumeActivity(ActivityThread.java:4250)
    at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1839)
    at android.os.Handler.dispatchMessage(Handler.java:102)
    at android.os.Looper.loop(Looper.java:158)
    at android.app.ActivityThread.main(ActivityThread.java:7229)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1230)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1120)
    Caused by: java.util.ConcurrentModificationException
    at java.util.ArrayList$ArrayListIterator.next(ArrayList.java:573)
    at com.app.android.view.fragment.MainFragment.runPendingNavigationActions(MainFragment.java:1355)

Does any Action remove itself from the list in its run() method? — Kevin Krumwiede
– Kevin Krumwiede, Commented Jun 16, 2016 at 6:46
No, run doesn't remove from the list but in some race condition the following code gets executed along while the loop is iterating /** * To ensure actions are done only in between onReusme and onPause, otherwise suspend them to be run once the app is resumed. */ private void runNavAction(Action action) { if (!canCommitFragmentTransaction) { pendingNavActions.add(action); } else { action.run(); } } — Bulu
– Bulu, Commented Jun 16, 2016 at 6:51
Have you solved this problem? If not, pls post the content of run() method in your Action class here — toantran
– toantran, Commented Jun 16, 2016 at 6:55
@Bulu If that code is running at the same time, that would explain the problem. Why is that code running in another thread? And what are these Actions? — Kevin Krumwiede
– Kevin Krumwiede, Commented Jun 16, 2016 at 7:03

Andy Turner · Accepted Answer · 2016-06-16 08:24:17Z

6

You are using the wrong data structure: a Queue would be a better choice, specifically one designed for concurrent modification.

For example, you can use LinkedBlockingQueue. You can add to this like a list, and then consume the added actions like:

Action r;
while ((r = queue.poll()) != null) {
  r.run();
}

Using Collections.synchronizedList would not fix your issue, at least not in and of itself.

The thing to bear in mind about wrapping a list with Collections.synchronizedList() is that it only provides mutual exclusion for each individual method call, e.g. until the call to pendingActions.get() or pendingActions.iterator() returns.

The enhanced for loop implicitly creates an Iterator; but the list is not then locked while you iterate over it, whether or not this comes from a synchronized list.

Your other thread is changing the list during this iteration, and would still cause the ConcurrentModificationException next time you try to get a value from the iterator.

Now, you could make it work (without the CME) by wrapping the iteration in a synchronized block as well:

synchronized (pendingActions) {
  for (Action r : pendingActions) {
    r.run();
  }
  r.clear();
}

The problem with this is that other threads will now be blocked whilst the list is being iterated. That might cause a noticeable delay in your app; or it might not, depending upon how frequently this method is called, and how long the actions take to run.

However, using a concurrent queue avoids that issue entirely, and is likely to be faster than a synchronized list too, since non-blocking concurrent queues don't require the use of "full-blown" synchronization.

edited Jun 16, 2016 at 8:24

answered Jun 16, 2016 at 7:55

Andy Turner

141k11 gold badges169 silver badges263 bronze badges

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

1 Comment

Bulu Over a year ago

Thanks for the great answer. I used LinkedBlockingQueue and hope should work. I have never been able to replicate the issue but seen in Production Crash logs and hope for the best to test in Prod and shouldn't introduce any new problems with the replacement for LinkedBlockingQueue with ArrayList

Collectives™ on Stack Overflow

java.util.ConcurrentModificationException - Android

Exception:

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

Exception:

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related