java.util.ConcurrentModificationException - ArrayList

Question

START EDIT please scroll down for the updated code END OF EDIT

I've google and searched around SO for why this exception is occurring and I understand that it is caused by an object is reading a list and meanwhile an item was removed from the list.

I've changed my code accordingly to the suggestions I've found but from time to time I still get this exception and it is crashing my app. And it looks randomly, I try to replicate the exception and 90% of the time I don't get the exception and not always following the same procedure, which makes it hard to debug.

I'm using the observer pattern. Sometimes it happens with the unregister method, some othertimes with the register, other times with a method from the notify... it's pretty random to where it happens.

I'm using an android asynctask to download few bytes from my server and the observer pattern is to update the GUI when needed.

Here's my code:

@Override
    public void register(final Observer newObserver) {
        Log.d(TAG, "(Register) Observer registred: " + newObserver.toString());
        observers.add(newObserver);

        Log.d(TAG, "(Register) Number of registered observers: " + observers.size());

    }

    @Override
    public void unregister(final Observer observer) {

        int indexObersver = observers.indexOf(observer);

        // Avoid java.util.ConcurrentModificationException 
        // at java.util.ArrayList$ArrayListIterator.next(ArrayList.java)

        if(indexObersver >= 0)
        {
            observers.remove(indexObersver);
            Log.d(TAG, "(Unregister) Unregistered Observer: " + observer.toString());
            Log.d(TAG, "(Unregister) Now we have: " + observers.size() + " observers");
        }
        else
        {
            Log.d(TAG, "(Unregister) Registered Observer not found");
        }
    }

    @Override
    public void notifyObserverNewLocalBackup(BackupInfo backupInfo) {

        // Avoid java.util.ConcurrentModificationException 
        // at java.util.ArrayList$ArrayListIterator.next(ArrayList.java)

        for( Iterator< Observer > it = observers.iterator(); it.hasNext() ; )
//      for(Observer observer : observers)
        {
            Observer observer = it.next();
            observer.notifyNewLocalBackup(backupInfo);
        }

    }

    @Override
    public void notifyObserverNewRemoteBackup(ArrayList<PhoneBackup> phoneBackups) {

        // Avoid java.util.ConcurrentModificationException 
        // at java.util.ArrayList$ArrayListIterator.next(ArrayList.java)

//      for(Observer observer : observers)
        for( Iterator< Observer > it = observers.iterator(); it.hasNext() ; )
        {
            Observer observer = it.next();
            observer.notifyNewRemoteBackup(phoneBackups);
        }
    }

    @Override
    public void notifyObserverDownloadCompleted(PhoneBackup phoneBackup) {

        // Avoid java.util.ConcurrentModificationException 
        // at java.util.ArrayList$ArrayListIterator.next(ArrayList.java)

//      for(Observer observer : observers)
        for( Iterator< Observer > it = observers.iterator(); it.hasNext() ; )
        {
            Observer observer = it.next();
            observer.notifyDownloadCompleted(phoneBackup);
        }

    }

    @Override
    public void notifyObserverUploadCompleted(boolean isSucccess) {

        // Avoid java.util.ConcurrentModificationException 
        // at java.util.ArrayList$ArrayListIterator.next(ArrayList.java)

//      for(Observer observer : observers)
        for( Iterator< Observer > it = observers.iterator(); it.hasNext() ; )
        {
            Observer observer = it.next();
            observer.notifyUploadCompleteted(isSucccess);
        }
    }

Now last time I got the excption it happened on notifyObserverNewRemoteBackup method at line Observer observer = it.next();

06-12 04:31:58.394: W/dalvikvm(31358): threadid=1: thread exiting with uncaught exception (group=0x418fcce0)
06-12 04:31:58.629: E/AndroidRuntime(31358): FATAL EXCEPTION: main
06-12 04:31:58.629: E/AndroidRuntime(31358): Process: com.mypackage.android.design.appdesgin, PID: 31358
06-12 04:31:58.629: E/AndroidRuntime(31358): java.util.ConcurrentModificationException
06-12 04:31:58.629: E/AndroidRuntime(31358):    at java.util.ArrayList$ArrayListIterator.next(ArrayList.java:573)
06-12 04:31:58.629: E/AndroidRuntime(31358):    at com.mypackage.android.design.appdesgin.asynctasks.ObserverSubjectManager.notifyObserverNewRemoteBackup(ObserverSubjectManager.java:99)
06-12 04:31:58.629: E/AndroidRuntime(31358):    at com.mypackage.android.design.appdesgin.asynctasks.BackupsHandler$1.success(BackupsHandler.java:318)
06-12 04:31:58.629: E/AndroidRuntime(31358):    at com.mypackage.android.design.appdesgin.asynctasks.BackupsHandler$1.success(BackupsHandler.java:1)
06-12 04:31:58.629: E/AndroidRuntime(31358):    at retrofit.CallbackRunnable$1.run(CallbackRunnable.java:45)
06-12 04:31:58.629: E/AndroidRuntime(31358):    at android.os.Handler.handleCallback(Handler.java:733)
06-12 04:31:58.629: E/AndroidRuntime(31358):    at android.os.Handler.dispatchMessage(Handler.java:95)
06-12 04:31:58.629: E/AndroidRuntime(31358):    at android.os.Looper.loop(Looper.java:136)
06-12 04:31:58.629: E/AndroidRuntime(31358):    at android.app.ActivityThread.main(ActivityThread.java:5081)
06-12 04:31:58.629: E/AndroidRuntime(31358):    at java.lang.reflect.Method.invokeNative(Native Method)
06-12 04:31:58.629: E/AndroidRuntime(31358):    at java.lang.reflect.Method.invoke(Method.java:515)
06-12 04:31:58.629: E/AndroidRuntime(31358):    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:791)
06-12 04:31:58.629: E/AndroidRuntime(31358):    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:607)
06-12 04:31:58.629: E/AndroidRuntime(31358):    at dalvik.system.NativeStart.main(Native Method)

---------------------- EDIT -------------------------------

I've followed Anubian Noob suggestion and I implemented a synchronized list but I'm still getting the exception.

Here's my updated code:

// Singleton
    public synchronized static ObserverSubjectManager getInstance()
    {
        if(instance == null)
        {
            instance = new ObserverSubjectManager();

            return instance;
        }
    return instance;
}


private ObserverSubjectManager()
{
//      observers = new ArrayList<>();  



    observers = Collections.synchronizedList(new ArrayList<Observer>());
}


@Override
public void register(final Observer newObserver) {
    Log.d(TAG, "(Register) Observer registred: " + newObserver.toString());

    synchronized (observers) {
        observers.add(newObserver);
    }


    Log.d(TAG, "(Register) Number of registered observers: " + observers.size());

}

@Override
public void unregister(final Observer observer) {

    synchronized (observers) 
    {
        int indexObersver = observers.indexOf(observer);

        if(indexObersver >= 0)
        {
            observers.remove(indexObersver);
            Log.d(TAG, "(Unregister) Unregistered Observer: " + observer.toString());
            Log.d(TAG, "(Unregister) Now we have: " + observers.size() + " observers");
        }
        else
        {
            Log.d(TAG, "(Unregister) Registered Observer not found");
        }
    }


}

@Override
public void notifyObserverNewLocalBackup(final BackupInfo backupInfo) {

    synchronized (observers) 
    {
        for(Observer observer : observers)
        {
            observer.notifyNewLocalBackup(backupInfo);
        }
    }


}

@Override
public void notifyObserverNewRemoteBackup(final ArrayList<PhoneBackup> phoneBackups) {

    synchronized (observers) 
    {
        for(Observer observer : observers)
        {
            observer.notifyNewRemoteBackup(phoneBackups);
        }
    }
}

@Override
public void notifyObserverDownloadCompleted(final PhoneBackup phoneBackup) {

    synchronized (observers) 
    {
        for(Observer observer : observers)
        {
            observer.notifyDownloadCompleted(phoneBackup);
        }
    }
}

@Override
public void notifyObserverUploadCompleted(final boolean isSucccess) {

    synchronized (observers) 
    {
        for(Observer observer : observers)
        {
            observer.notifyUploadCompleteted(isSucccess);
        }
    }
}

Stacktrace:

06-12 05:12:49.359: W/dalvikvm(31735): threadid=1: thread exiting with uncaught exception (group=0x418fcce0)
06-12 05:12:49.426: E/AndroidRuntime(31735): FATAL EXCEPTION: main
06-12 05:12:49.426: E/AndroidRuntime(31735): Process: com.mypackage.android.design.appdesgin, PID: 31735
06-12 05:12:49.426: E/AndroidRuntime(31735): java.util.ConcurrentModificationException
06-12 05:12:49.426: E/AndroidRuntime(31735):    at java.util.ArrayList$ArrayListIterator.next(ArrayList.java:573)
06-12 05:12:49.426: E/AndroidRuntime(31735):    at com.mypackage.android.design.appdesgin.asynctasks.ObserverSubjectManager.notifyObserverDownloadCompleted(ObserverSubjectManager.java:126)
06-12 05:12:49.426: E/AndroidRuntime(31735):    at com.mypackage.android.design.appdesgin.asynctasks.BackupsHandler$2.success(BackupsHandler.java:336)
06-12 05:12:49.426: E/AndroidRuntime(31735):    at com.mypackage.android.design.appdesgin.asynctasks.BackupsHandler$2.success(BackupsHandler.java:1)
06-12 05:12:49.426: E/AndroidRuntime(31735):    at retrofit.CallbackRunnable$1.run(CallbackRunnable.java:45)
06-12 05:12:49.426: E/AndroidRuntime(31735):    at android.os.Handler.handleCallback(Handler.java:733)
06-12 05:12:49.426: E/AndroidRuntime(31735):    at android.os.Handler.dispatchMessage(Handler.java:95)
06-12 05:12:49.426: E/AndroidRuntime(31735):    at android.os.Looper.loop(Looper.java:136)
06-12 05:12:49.426: E/AndroidRuntime(31735):    at android.app.ActivityThread.main(ActivityThread.java:5081)
06-12 05:12:49.426: E/AndroidRuntime(31735):    at java.lang.reflect.Method.invokeNative(Native Method)
06-12 05:12:49.426: E/AndroidRuntime(31735):    at java.lang.reflect.Method.invoke(Method.java:515)
06-12 05:12:49.426: E/AndroidRuntime(31735):    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:791)
06-12 05:12:49.426: E/AndroidRuntime(31735):    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:607)
06-12 05:12:49.426: E/AndroidRuntime(31735):    at dalvik.system.NativeStart.main(Native Method)

My guess is that another thread might be registering/unregistering an observer, which triggers the ConcurrentModificationException. Do you see anything in your log about an observer being registered/unregistered before the ConcurrentModificationException? — awksp
– awksp, Commented Jun 12, 2014 at 15:48
Personally I would say you're iterating and changing the contents of the arraylist during the iteration. Though I can't see where with the amount of code you've given now — Rogue
– Rogue, Commented Jun 12, 2014 at 15:49
For starters, just use an enhanced for loop. That's not your problem, but your code is more difficult to read because of doing it manually. — chrylis -cautiouslyoptimistic-
– chrylis -cautiouslyoptimistic-, Commented Jun 12, 2014 at 16:03
@chrylis I was using the enhanced for loop, as you can see in the comments but from what I read on the internet and SO it was advised to use iterators and use the iterators to access and modify the list — dazito
– dazito, Commented Jun 12, 2014 at 16:06

Kevin Coppock · Accepted Answer · 2014-06-12 17:11:48Z

8

To follow up @Rogue's comment, I would look for any instances where any of your notify (notifyDownloadCompleted(), etc.) callback implementations unregister an observer. What can easily happen is that:

1) You're iterating over a collection. While in that iteration, you call a method on one of the registered observers.

2) That registered observer, in the notify callback, calls through to unregister itself from further notifications.

3) Since you're still in that iteration loop, this will cause a ConcurrentModificationException as you cannot modify a collection while iterating over it.

You could fix this by doing a reverse loop:

for (int i = collection.size() - 1; i >= 0; i--) {
    collection.get(i).notifyDownloadCompleted();
}

Although you could technically still run into some edge cases there, but not an exception.

answered Jun 12, 2014 at 17:11

Kevin Coppock

135k45 gold badges270 silver badges279 bronze badges

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

1 Comment

dazito Over a year ago

I'm accepting this answer as it was the closest one to my problem. My problem was indeed me iterating over the collection while an object unregistered himself. This threading hassle made me to move over to EventBus library form greenrobot. Super flexible and easy to use, I'm in love with it! No more need to use the observer pattern with threads, in this case.

Anubian Noob · Accepted Answer · 2014-06-12 15:50:20Z

4

The problem is that you're accessing your ArrayList from another thread, which means when you modify it you get that exception. An easy fix is to replace your ArrayList with a CopyOnWriteArrayList (which is a lot slower), or to use Collections.synchronizedList().

To make a synchronized list:

List<Observer> list = Collection.synchronizedList(new ArrayList<Observer>);

answered Jun 12, 2014 at 15:50

Anubian Noob

13.6k6 gold badges55 silver badges76 bronze badges

3 Comments

awksp Over a year ago

Might want to note that just dropping in Collections.synchronizedList() won't allow proper concurrent behavior by itself.

dazito Over a year ago

I also thought the same, some thread could be modifying it. So I did synchronized (observer) {the for cycles here} and still got that exception, so I'm confused

dazito Over a year ago

@Anubian Noob can you please check my edited question. I've edited my code to use a synchronizedList but I'm still getting the exception

Stefan Walter · Accepted Answer · 2014-06-12 17:31:57Z

3

If you are not accessing the collection from multiple threads, but only want to avoid problems when changing the collection while iterating over it, probably the easiest way is to iterate over a copy of your collection instead:

for (Observer observer : new ArrayList<>(observers)) {
  observer.notifyNewLocalBackup(backupInfo);
}

This implies a certain overhead for creating the copy, of course.

You can also use a CopyOnWriteArrayList, which covers the case of access from concurrent threads, too.

answered Jun 12, 2014 at 17:31

Stefan Walter

6223 silver badges10 bronze badges

Comments

RatneZ · Accepted Answer · 2019-03-29 07:42:13Z

0

Use Iterator inside of your for/foreach loop

List<String> stringArrayList = new ArrayList<>();
for (Iterator<String> stringIterator = stringArrayList.iterator(); 
 stringIterator.hasNext(); ) {
   String string = stringIterator.next();
   if (string.equalsIgnoreCase("otherString")) {
   stringIterator.remove();
   }
 }

P.S. You can simplify the above code using this lambda expression

 stringArrayList.removeIf(string -> string.equalsIgnoreCase("otherString"));

answered Mar 29, 2019 at 7:42

RatneZ

1,20810 silver badges9 bronze badges

1 Comment

Allen Over a year ago

Hi @RatneZ, thanks for the answer. May I know why using an iterator can solve the issue?

Collectives™ on Stack Overflow

java.util.ConcurrentModificationException - ArrayList

4 Answers 4

1 Comment

3 Comments

Comments

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

3 Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related