2

I am trying to update ArrayList of Locations as per below logic in a service, running in background thread through Runnable.I have edited code to show only relevant code. I am getting ConcurrentModificationException.

public static ArrayList<Location> locationPoints;


@Override
public void onLocationChanged(final Location location) {
    //Log.i(TAG, "onLocationChanged: " + location);
    Log.i(TAG, "onLocationChanged: " + locationPoints.size());

    ArrayList<Location> alnew= locationPoints;

    if(!locationPoints.isEmpty()){
        for(Location l:alnew){
            if(location.distanceTo(l)<=200.0f){
                locationPoints.add(l);
            }else{
                locationPoints.add(location);
            }
        }
    }else{
        locationPoints.add(location);
    }

    sendLocationsToActivity(locationPoints);
}

I want location objects going one after the other, but i see location objects increasing exponentially.

Result from Log in onLocationChanged is as below. Exception is emanating from ArrayList methods but I am unable to find a solution even after using all remedies given here.

onLocationChanged: 0
onLocationChanged: 1
onLocationChanged: 2
onLocationChanged: 4
onLocationChanged: 8
onLocationChanged: 16
onLocationChanged: 32

But if I delete all the ArrayList logic from onLocationChanged and simply add location objects to ArrayList, the results are the way I want

onLocationChanged: 0
onLocationChanged: 1
onLocationChanged: 2
onLocationChanged: 3
onLocationChanged: 4
onLocationChanged: 5

StackTrace:

FATAL EXCEPTION: main
Process: com.amreen.test, PID: 27053
Theme: themes:{default=overlay:com.resurrectionremix.pitchblack, fontPkg:com.resurrectionremix.pitchblack, com.android.systemui=overlay:com.resurrectionremix.pitchblack, com.android.systemui.headers=overlay:com.resurrectionremix.pitchblack, com.android.systemui.navbar=overlay:com.resurrectionremix.pitchblack}
java.util.ConcurrentModificationException
    at java.util.ArrayList$ArrayListIterator.next(ArrayList.java:573)
    at com.amreen.test.MyLocationService.onLocationChanged(MyLocationService.java:146)
    at com.google.android.gms.location.internal.zzk$zzb.handleMessage(Unknown Source)
    at android.os.Handler.dispatchMessage(Handler.java:102)
    at android.os.Looper.loop(Looper.java:148)
    at android.app.ActivityThread.main(ActivityThread.java:5458)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616)
Tom
  • 16,842
  • 17
  • 45
  • 54
  • Please provide a (short) stack-trace of the exception, and please try to minimize your code and provide a [*Minimal*, Complete, and Verifiable example](http://stackoverflow.com/help/mcve). Did you search for other ConcurrentModificationException questions? There are lots of resources on this topic. – Martin Nyolt Nov 10 '16 at 08:39
  • I have made changes as instructed. – user7090887 Nov 10 '16 at 08:52
  • 1
    ConcurrentModificationException occurs when you modify the list (adding or removing elements) while traversing a list with Iterator. Here the for-each loop internally makes use of iterator. You need to use another arraylist and keep the elements in that and after exiting from loop update the original collection. – akhil_mittal Nov 10 '16 at 08:56
  • I tried that but I still got same exception and result. – user7090887 Nov 10 '16 at 08:57
  • Please paste the updated code if you have tried that. – akhil_mittal Nov 10 '16 at 08:59
  • I tried CopyOnWriteArrayList() as well. it helped to eradicate exception but results were same increasing exponentially. Ultimately causing outofmemory exception. – user7090887 Nov 10 '16 at 09:00
  • I pasted some sample to show what I mean. Just in case. – akhil_mittal Nov 10 '16 at 09:04
  • 1
    Check the other question + consider making your list thread safe with [`Collections.synchronizedList(list)`](https://docs.oracle.com/javase/8/docs/api/java/util/Collections.html#synchronizedList-java.util.List-) – Nicolas Filotto Nov 10 '16 at 09:11
  • 1
    Your for-loop always adds a new location (or copies a previous one) as many times as there are locations in the collection. Of course the collection then grows exponentially. I don't know what's your exact idea, but probably comparing the new location to **all** the previous locations isn't what you meant to do. – Markus Kauppinen Nov 10 '16 at 10:35
  • changed my code after learning from your comment. check my answer. – user7090887 Nov 10 '16 at 16:47
  • ListIterator iter = locationPoints.listIterator(); if (location.distanceTo(locationPoints.get(locationPoints.size() - 1)) >= 90.0f) { while (iter.hasNext()) { Location l = iter.next(); if (location.distanceTo(l) <= 80.0f) { pointsToBeAdd.add(l); } else { pointsToBeAdd.add(location); } } } – user7090887 Nov 10 '16 at 16:47
  • Stop manipulating your code. If you have a new issue/question, then create a new post. – Tom Nov 11 '16 at 20:47

2 Answers2

2

ConcurrentModificationException occurs when you modify the list (adding or removing elements) while traversing a list with Iterator. The for-each loop is nothing but syntactic sugar for java.util.Iterator. So here the logic is like:

for() {
     if(someCondition) {
         locationPoints.add(sth);
   }
}

Rather than that you can try sth like:

for() {
         if(someCondition) {
             someOtherArrayList.add(sth);
       }
    }

And once out of the loop and all then:

locationPoints.addAll(someOtherArrayList);
akhil_mittal
  • 23,309
  • 7
  • 96
  • 95
  • I have updated the code but problem still persists, please check if i made new arrayList alnew. But while writing this comment, I realise i made a mistake of referring to same object. I will try as per your instruction. – user7090887 Nov 10 '16 at 09:11
  • your method solved exception but weird results still continues – user7090887 Nov 10 '16 at 10:16
1

It is because the ArrayList implementation, please refer to the document ArrayList document

The iterators returned by this class's iterator and listIterator methods are fail-fast: if the list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException. Thus, in the face of concurrent modification, the iterator fails quickly and cleanly, rather than risking arbitrary, non-deterministic behavior at an undetermined time in the future.

You bet firstly get the iterator, and then use it for add. Example

Iterator<Location> iter = locationPoints.iterator(); while (iter.hasNext()) { Location location = iter.next(); if(location.distanceTo(l)<=200.0f){ iter.add(l); }else{ iter.add(location); } }

kidnan1991
  • 366
  • 2
  • 12