Take the 2-minute tour ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free, no registration required.

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)
share|improve this question
    
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? –  user3580294 Jun 12 at 15:48
1  
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 Jun 12 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 Jun 12 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 –  dwnz Jun 12 at 16:06

3 Answers 3

up vote 2 down vote accepted

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.

share|improve this answer
    
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. –  dwnz Jun 13 at 10:14

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>);
share|improve this answer
    
Might want to note that just dropping in Collections.synchronizedList() won't allow proper concurrent behavior by itself. –  user3580294 Jun 12 at 15:52
    
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 –  dwnz Jun 12 at 15:54
    
@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 –  dwnz Jun 12 at 16:20

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.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.