What does the stacktrace mean?

Question

E/AndroidRuntime: FATAL EXCEPTION: main
                  Process: dev.game.adventure, PID: 21482
                  java.util.ConcurrentModificationException
                      at java.util.LinkedList$ListItr.checkForComodification(LinkedList.java:966)
                      at java.util.LinkedList$ListItr.next(LinkedList.java:888)
                      at dev.game.adventure.Troll.wakeup(Troll.java:32)
                      at dev.game.adventure.Simulation$1.run(Simulation.java:29)
                      at android.os.Handler.handleCallback(Handler.java:789)
                      at android.os.Handler.dispatchMessage(Handler.java:98)
                      at android.os.Looper.loop(Looper.java:164)
                      at android.app.ActivityThread.main(ActivityThread.java:6541)
                      at java.lang.reflect.Method.invoke(Native Method)
                      at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:240)
                      at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:767)

I got the above trace after running in the emulator for some time. What does it mean? How can I resolve it? I can't even reproduce it because it only happens after some time. This is the offending code:

thing = i.next(); (Troll.java:32)

Class Troll.java

package dev.game.adventure;

import android.os.Handler;
import android.widget.Toast;

import java.util.*;

public class Troll extends WalkingPerson {

    private boolean hasCoke = false;
    private AdventureActivity target;

    Troll(Simulation s, World world, String name, AdventureActivity app) {
        super(s, world, name, R.mipmap.lobgoblin, app);
        this.target = app;
        goTo("Kulverten", target);
    }

    public void wakeup() {

        Person victim = (Person) (place.persons.toArray()[((int) (Math.random() * place.persons
                .size()))]);


        if (victim instanceof Troll) { // Don't commit suicide
        } else {
            say("Victim is " + victim.getName() + ".", target);
            Thing thing;

            for (Iterator<Thing> i = victim.things.iterator(); i.hasNext(); ) {

                thing = i.next();

                if (thing instanceof CocaCola) {
                    hasCoke = true;
                    victim.things.remove(thing);
                    world.update(place, target);
                    say("Now I take your Coca Cola!", target);
                    //TODO: Actually take the drink
                } else {
                    drop(thing.name, target); // drop all items so that the key doesn't end up in heaven
                }
            }

            if (!hasCoke) {
                say("Now I kill you!", target); // TODO: Take all the items of the victim
                //  this.getWorld().playAtPlace(place, "effects/gong");
                final Person victim2 = victim;
                Handler handler = new Handler();
                handler.postDelayed(new Runnable() {
                    @Override
                    public void run() {
                        victim2.goTo("Heaven", target);
                        world.update(victim2.place, target);
                        world.sayAtPlace(victim2.place, "-- Game Over --", target);
                        if (victim2.getName().equals("You")) {
                            target.ag.setBackground(world.getPlace("Heaven").getImage());
                            final Toast toast = Toast.makeText(target.ag.getContext(), "GAME OVER!\n", Toast.LENGTH_LONG);// duration);
                            toast.show();
                        }

                    }
                }, 1500);
            }
        }
        Handler handler = new Handler();
        handler.postDelayed(new Runnable() {
            @Override
            public void run() {
                if(Math.random() > 0.3)
                    go(place.randomDoor(), target);
            }
        }, 2500);
        s.wakeMeAfter(this, (80 + Math.random() * 1000));
    }
}

Answer 1

Replace:

victim.things.remove(thing);

With:

i.remove();

The only safe way to remove an item from a list you are iterating is with the iterator.

If you do it with the list.remove() method, it would be difficult for the iterator to update its internal state to handle the deletion correctly. As such, it just "gives up" - it throws the ConcurrentModificationException to say "I'm in an invalid state, I can't recover from this".

---

Incidentally, I think the reason you only see this sporadically is because of the ordering of items in the list.

The implementation of hasNext in LinkedList.Itr simply checks:

return nextIndex < size;

Where nextIndex is a member variable of the iterator which is updated in next() to be the index of the next element to be returned.

If you remove an item with list.remove, the size decreases. So, if the item you removed with list.remove happened to be the last item in the list (or the penultimate item), hasNext() is false, and so you don't call next() and get the CME.

It is worth noting that if you did remove the penultimate element, the loop would break before processing the final item in the list, without any exception. This is insidious behaviour, and would be a source of a real head-scratcher of a bug. You can avoid it by only removing from the list via the iterator.

Answer 2

You can simply reproduce this exception by:

ArrayList<Integer> list = new ArrayList<>(Arrays.asList(1,2,3,5));
Iterator<Integer> iter = list.iterator();
list.remove(0);
iter.next();

Basically, you cannot iterate over a list using an iterator while removing things from the list at the same time.

// 1. you have an iterator here
for (Iterator<Thing> i = victim.things.iterator(); i.hasNext(); ) {

    // 3. on the next iteration, you read from i and BOOM!
    thing = i.next();

    if (thing instanceof CocaCola) {
        hasCoke = true;
        // 2. you removed a thing here
        victim.things.remove(thing);
        world.update(place, target);
        say("Now I take your Coca Cola!", target);
        //TODO: Actually take the drink
    } else {
        drop(thing.name, target); // drop all items so that the key doesn't end up in heaven
    }
}

To actually remove stuff, you should call remove on the iterator:

i.remove() // replace things.remove with this