ConcurrentModificationException when using iterator and iterator.remove()

Question

    private int checkLevel(String bigWord, Collection<String> dict, MinMax minMax)
{
    /*value initialised to losing*/
    int value = 0; 
    if (minMax == MinMax.MIN) value = 1; 
    else value = -1; 


    boolean go = true;

    Iterator<String> iter = dict.iterator();

    while(iter.hasNext())
    {
        String str = iter.next(); 
        Collection<Integer> inds = naiveStringSearch(bigWord, str);

        if(inds.isEmpty())
        {
            iter.remove();
        }

        for (Integer i : inds)
        {
            MinMax passin = minMax.MIN;
            if (minMax == MinMax.MIN) passin = minMax.MAX;

            int value2 = checkLevel(removeWord(bigWord, str, i), dict, passin); 
            if (value2 == -1 && minMax == minMax.MIN)
            {
                value = -1; 
                go = false;
            }
            if (value2 == 1 && minMax == minMax.MAX)
            {
                value = 1; 
                go = false; 
            }

        }

        if (go == false) break; 
    }


    return value;
}

Error:

Exception in thread "main" java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(HashMap.java:810)
at java.util.HashMap$KeyIterator.next(HashMap.java:845)
at aStringGame.Main.checkLevel(Main.java:67)
at aStringGame.Main.test(Main.java:117)
at aStringGame.Main.main(Main.java:137)

What's the problem here?

@Nambari - I've updated the code to show the entire method. It's a recursive method. — dwjohnston
– dwjohnston, Commented Dec 3, 2012 at 22:14

NPE · Accepted Answer · 2012-12-03 22:17:21Z

5

Something somewhere is modifying dict. I suspect it might be happening inside this call:

int value2 = checkLevel(removeWord(bigWord, str, i), dict, passin);
                                                     ^^^^

edit Basically, what happens is that the recursive call to checkLevel() modifies dict through another iterator. This makes the outer iterator's fail-fast behaviour to kick in.

edited Dec 3, 2012 at 22:17

answered Dec 3, 2012 at 22:11

NPE

503k114 gold badges970 silver badges1k bronze badges

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

5 Comments

dwjohnston Over a year ago

^I've updated the question to show that it's a recursive method. What would the best solution be? Cloning the dictionary I'm passing in?

dwjohnston Over a year ago

@jahroy - but the collection is a hashset. (The reason for it being a hashset is that I'm concerned about performance, I'm not sure that a hashset will be anyfast for simple iterating, and removing elements but).

Peter Lawrey Over a year ago

If you are iterating, all collections are equal in terms of performance. BTW if you use a Concurrent Set you won't have this isssue.

jahroy Over a year ago

A Set would improve performance by ensuring there are no duplicates. If you create a List from a HashSet, the List will also have no duplicates.

jahroy Over a year ago

@user1068446 - If you're worried about converting between Set and List, check out the second half of my answer.

jahroy · Accepted Answer · 2012-12-03 22:54:16Z

4

You can't modify a Collection while you're iterating over it with an Iterator.

Your attempt to call iter.remove() breaks this rule (your removeWord method might, too).

You CAN modify a List while iterating IF you use a ListIterator to iterate.

You can convert your Set to a List and use a List iterator:

List<String> tempList = new ArrayList<String>(dict);
ListIterator li = tempList.listIterator();

Another option is to keep track of the elements you want to remove while iterating.

You could place them in a Set, for example.

You could then call dict.removeAll() after your loop.

Example:

Set<String> removeSet = new HashSet<String>();
for (String s : dict) {
    if (shouldRemove(s)) {
        removeSet.add(s);
    }
}
dict.removeAll(removeSet);

edited Dec 3, 2012 at 22:54

answered Dec 3, 2012 at 22:11

jahroy

22.8k10 gold badges62 silver badges111 bronze badges

Comments

andreih · Accepted Answer · 2012-12-03 22:13:02Z

1

When using a for each loop you are not allowed to modify the Collection you are iterating inside the loop. If you need to modify it, use a classic for loop

answered Dec 3, 2012 at 22:13

andreih

4331 gold badge6 silver badges19 bronze badges

1 Comment

jahroy Over a year ago

It's true that a traditional for loop will avoid a ConcurrentModificationException. However, you can't access elements in a Set by their index.

CodeDreamer · Accepted Answer · 2012-12-03 22:13:28Z

1

This is a common occurance in all Collections classes. For instance the entry in TreeSet uses failfast method.

The iterators returned by this class's iterator method are fail-fast: if the set is modified at any time after the iterator is created, in any way except through the iterator's own remove method, 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.

http://docs.oracle.com/javase/6/docs/api/java/util/TreeSet.html

answered Dec 3, 2012 at 22:13

CodeDreamer

4442 silver badges9 bronze badges

Collectives™ on Stack Overflow

ConcurrentModificationException when using iterator and iterator.remove()

4 Answers 4

5 Comments

Comments

1 Comment

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

5 Comments

Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related